refactor(map): migrate custom overlays to the maps-utils 5.0 data layer#6304
Conversation
android-maps-utils 5.0.0 (pulled by maps-compose 8.4.0) deprecated the com.google.maps.android.data KML/GeoJSON stack. Crucially, those deprecated classes are no longer the battle-tested 4.x code: they are thin bridges over the new platform-agnostic pipeline, and the bridges regress this feature — GeoJSON/KML point markers render invisible (PointStyle(color=0) maps to marker alpha 0), per-feature MultiPolygon styles are dropped at render time (Site Planner coverage contours lose their dBm colors), and MultiGeometry features cannot be removed (layer toggle-off leaks objects, re-show duplicates them). Rebuild the Map Layers region directly on the new public pipeline instead: - Parse with GeoJsonParser / KmlParser / KmzParser (zip magic sniffed, since LayerType.KML covers both .kml and .kmz) and map via toLayer(). - Re-apply simplestyle-spec styling as a pass over the immutable model features, preserving everything the upstream mapper misses: the legacy `color` fallback, rgb()/rgba() colors, the 0.35 default fill opacity that makes stacked contour bands read as a gradient, and PolygonStyle for MultiPolygon (upstream styles any multi-geometry as a line). - Render through a single-use MapViewRenderer per show, torn down with clear() — the only teardown that reliably removes MultiGeometry objects — re-wiring KMZ images via cacheImageData on each show. Behavior preserved: KML/KMZ/GeoJSON custom layers, per-feature simplestyle colors, visibility toggling, refresh re-parse, points keep default markers. Verified: spotlessApply spotlessCheck detekt :androidApp:compileGoogleDebugKotlin (zero deprecation warnings remain in map code). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesMap rendering pipeline
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MapLayerOverlay
participant parseMapLayer
participant MapViewRenderer
MapLayerOverlay->>parseMapLayer: parse GeoJSON, KML, or KMZ
parseMapLayer->>MapViewRenderer: provide DataLayer
MapViewRenderer->>MapLayerOverlay: render or clear map layer
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@androidApp/src/google/kotlin/org/meshtastic/app/map/MapView.kt`:
- Around line 1432-1448: Update Feature.applySimpleStyleSpec() to parse the
stroke-opacity property and apply it to the resolved stroke color before
rebuilding styles. Use the alpha-adjusted stroke color for both
PolygonStyle.strokeColor and LineStyle.color, while preserving existing
fill-opacity and default-color behavior.
- Around line 1316-1324: Move the parseMapLayer call out of the MapEffect
execution context so KML/KMZ/GeoJSON parsing runs on the injected IO dispatcher.
Prefer exposing this through MapLayersManager or MapViewModel; otherwise wrap
only the inputStream.use { parseMapLayer(...) } step with the IO dispatcher,
while preserving the existing null and RenderedMapLayer handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b7586055-bc2d-448b-b7f3-385de00b0902
📒 Files selected for processing (1)
androidApp/src/google/kotlin/org/meshtastic/app/map/MapView.kt
Address CodeRabbit review on #6304: - Wrap the KML/KMZ/GeoJSON parse in Dispatchers.IO so large layers don't stall the main thread inside MapEffect (stream open already ran on IO; the parse itself did not). - Carry simplestyle stroke-opacity through the rebuilt styles for both PolygonStyle.strokeColor and LineStyle.color, matching the upstream mapper's handling that the rebuild replaces. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Follow-up to #6301. That bump pulled android-maps-utils 5.0.0, which deprecated the entire
com.google.maps.android.dataKML/GeoJSON stack ("Use the new platform-agnostic data layer and renderer instead"). The catch: in 5.0.0 the deprecated classes are no longer the battle-tested 4.x implementations — they are thin bridges over the new pipeline, and the bridges break this feature at runtime. This PR moves the google-flavor custom map overlays (Map Layers) directly onto the new public pipeline, which both clears the deprecation and fixes the regressions the bridges would ship.🐛 Bug Fixes (regressions in the 5.0.0 legacy bridges, avoided by migrating)
PointStyle(color = 0), which the renderer turns into marker alpha 0.MapViewRenderer.removeFeaturekeys rendered map objects by internal per-geometry copies in anIdentityHashMap, so removal no-ops and re-showing duplicates the geometry.🛠️ Refactoring & Architecture
MapLayerOverlaynow parses via the newGeoJsonParser/KmlParser/KmzParser(zip magic sniffed —LayerType.KMLcovers both.kmland.kmz) and maps into the immutableDataLayermodel viatoLayer().applySimpleStyleSpecis reimplemented as a pass over the model features, preserving everything the upstream mapper misses: the legacycolorfallback,rgb()/rgba()colors, the 0.35 default fill opacity that lets stacked contour bands read as a gradient, andPolygonStylefor MultiPolygon (upstream styles any multi-geometry as a line).MapViewRendererper show, torn down withclear()— the only teardown that reliably removes MultiGeometry objects — re-wiring KMZ archive images viacacheImageDataon each show.@Deprecatedmaps-utils usages remain in the map code.Behavior preserved: KML/KMZ/GeoJSON custom layers, per-feature simplestyle colors for Site Planner coverage exports, layer visibility toggling, refresh re-parse, points keep the default marker.
The three bridge/renderer bugs above are candidates for upstream reports against googlemaps/android-maps-utils.
Testing Performed
./gradlew spotlessApply spotlessCheck detekt assembleDebug test allTests— green.:androidApp:compileGoogleDebugKotlin— no deprecation warnings remain in map code.🤖 Generated with Claude Code
Summary by CodeRabbit