Repository navigation
feat(utils): add Compose wrappers for HeatmapTileOverlay, DataLayer, GeoJsonLayer, and KmlLayer - #1048
feat(utils): add Compose wrappers for HeatmapTileOverlay, DataLayer, GeoJsonLayer, and KmlLayer#1048dkhawk wants to merge 2 commits into
Conversation
Coverage (unit tests)Compared against
Line and branch coverage from unit test reports. History is recorded on the |
Coverage (instrumentation tests)
Files
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| val currentRenderer = renderer ?: return | ||
|
|
||
| DisposableEffect(currentRenderer, scene, useAdvancedMarkers) { |
There was a problem hiding this comment.
MapViewRenderer.clear() cancels its rendererScope for good (maps-utils 6.1.0), and here we keep rendering with the same instance after scene or useAdvancedMarkers change. From then on the loadIcon coroutines never run, so remote KML/GeoJSON icons stop loading. Could we create a new renderer per scene, or change clear() in maps-utils so it doesn't cancel the scope?
| @Composable | ||
| @GoogleMapComposable | ||
| @MapsComposeExperimentalApi | ||
| public fun DataLayer( |
There was a problem hiding this comment.
This adds ten public composables and several overlap (DataLayer(assetName) vs KmlLayer(assetName), DataLayer(rawResourceId) vs the raw resource GeoJsonLayer/KmlLayer), and once released they are hard to remove. My two cents: let's start with DataScene and DataLayer(layer) plus a single loading path, and add the format specific ones when someone asks for them. Naming them like the model classes also forces users to import both in the same file, as happens here.
| if (weightedData.isEmpty()) return | ||
|
|
||
| var holder by remember { mutableStateOf<HeatmapProviderHolder?>(null) } | ||
| val currentHolder = updateHeatmapProviderHolder( |
There was a problem hiding this comment.
updateHeatmapProviderHolder mutates the provider and writes holder during composition, so a discarded composition would leave the provider with values that were never committed. What about keeping the provider in a remember and pushing the updates from a SideEffect? Also, Gradient has no equals, so an inline Gradient(...) would call setGradient and clearTileCache() on every recomposition.
| */ | ||
| @Composable | ||
| @GoogleMapComposable | ||
| public fun HeatmapTileOverlay( |
There was a problem hiding this comment.
nit: the other new composables are @MapsComposeExperimentalApi, I would mark this one too (and the weighted overload) for consistency.
| public fun HeatmapTileOverlay( | |
| @MapsComposeExperimentalApi | |
| public fun HeatmapTileOverlay( |
| onLayerLoaded: (DataLayer?) -> Unit = {}, | ||
| ) { | ||
| val onLayerLoadedState by rememberUpdatedState(onLayerLoaded) | ||
| var loadedLayer by remember(geoJsonData) { mutableStateOf<DataLayer?>(null) } |
There was a problem hiding this comment.
JSONObject doesn't override equals, so an object built during composition is parsed again on every recomposition. Resetting loadedLayer to null on a key change also makes the layer flicker until the new one is parsed (same in the other loaders), so I would keep the previous layer until the new one is ready.
| import java.io.InputStream | ||
| import java.nio.charset.StandardCharsets | ||
|
|
||
| private const val DEFAULT_MAX_KMZ_ENTRY_COUNT = 200 |
There was a problem hiding this comment.
These copy the private defaults of KmzParser and can easily drift, shouldn't we expose them from maps-utils instead? The sniffing below also duplicates what DataLayerLoader already does.
| maxKmzUncompressedTotalSize = maxKmzUncompressedTotalSize, | ||
| ) | ||
| } | ||
| }.getOrNull() |
There was a problem hiding this comment.
With getOrNull() a missing resource, a malformed file and a KMZ rejected by the zip bomb limits all end up as onLayerLoaded(null), so apps can't tell them apart. I wonder if we could expose the error, e.g. through an onError callback.
37f6f7b to
b1d9f6a
Compare
…GeoJsonLayer, and KmlLayer - Add HeatmapTileOverlay composable wrapping HeatmapTileProvider and MapApplier tile overlays with reactive updates for data, weightedData, gradient, radius, opacity, and maxIntensity. - Add DataScene and DataLayer composables wrapping the platform-agnostic DataLayer, DataScene, and MapViewRenderer introduced in android-maps-utils 6.x. - Add GeoJsonLayer and KmlLayer composables supporting raw resources, JSONObject, InputStream, and KMZ archive security limits (maxKmzEntryCount, maxKmzUncompressedTotalSize). - Add unit tests for HeatmapTileOverlay and DataLayer/GeoJsonLayer/KmlLayer.
b1d9f6a to
c0d81bc
Compare
Stack
feat(utils): update android-maps-utils to 6.1.0 and expose clustering executors/contexts(Base →main)feat(utils): add Compose wrappers for HeatmapTileOverlay, DataLayer, GeoJsonLayer, and KmlLayer(Stacked →feat/maps-utils-6.1.0)Summary
HeatmapTileOverlay: Compose wrapper aroundHeatmapTileProviderandMapAppliertile overlays with reactive updates fordata,weightedData,gradient,radius,opacity, andmaxIntensity.DataScene&DataLayer: Compose wrappers around the platform-agnosticcom.google.maps.android.data.DataLayer,DataScene, andMapViewRendererintroduced inandroid-maps-utils6.x.GeoJsonLayer: Compose wrapper supporting raw GeoJSON resources,JSONObject, andInputStreamparsing viaDataLayerLoader.fromGeoJson.KmlLayer: Compose wrapper supporting raw KML/KMZ resources andInputStreamparsing viaDataLayerLoader.fromKml/DataLayerLoader.fromKmz, including the KMZ zip-bomb security limits (maxKmzEntryCount,maxKmzUncompressedTotalSize) introduced inandroid-maps-utils6.1.0 (fix(data): apply StyleMap styles and fix four KML and GeoJSON style bugs android-maps-utils#1809).HeatmapTileOverlayTest.ktandDataLayerTest.kt.