From a038104f6c8e63c86f197863a45279f7a29155b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20Lo=CC=81pez=20Man=CC=83as?= Date: Thu, 1 Oct 2026 18:51:23 +0200 Subject: [PATCH] fix(data): apply StyleMap styles and fix four KML and GeoJSON style bugs - KmlLayer resolved StyleMap entries with their leading '#' while shared styles are keyed by bare id, so placemarks styled through a StyleMap were unstyled. - KmlLayer no longer throws on a shared Style or StyleMap without an id; they are skipped since nothing can reference them. - KmlLayer no longer throws on a GroundOverlay without a LatLonBox (for example gx:LatLonQuad); the overlay is skipped with a warning. - GeoJsonLineStringStyle.toPolylineOptions now keeps the pattern and the start and end caps. - KmlStyle random icon color mode now randomizes the marker color instead of its hue, so markers keep their color family. The regression tests from the data coverage tests are no longer ignored. --- .../data/geojson/GeoJsonLineStringStyle.kt | 8 ++-- .../google/maps/android/data/kml/KmlLayer.kt | 42 ++++++++++++++----- .../google/maps/android/data/kml/KmlStyle.kt | 6 ++- .../android/data/geojson/GeoJsonStylesTest.kt | 7 +--- .../android/data/kml/KmlLayerParsingTest.kt | 17 +++----- .../maps/android/data/kml/KmlStyleTest.kt | 8 +--- 6 files changed, 51 insertions(+), 37 deletions(-) diff --git a/data/src/main/java/com/google/maps/android/data/geojson/GeoJsonLineStringStyle.kt b/data/src/main/java/com/google/maps/android/data/geojson/GeoJsonLineStringStyle.kt index 37cb056e7..f62f7b955 100644 --- a/data/src/main/java/com/google/maps/android/data/geojson/GeoJsonLineStringStyle.kt +++ b/data/src/main/java/com/google/maps/android/data/geojson/GeoJsonLineStringStyle.kt @@ -85,9 +85,11 @@ public class GeoJsonLineStringStyle : visible(mPolylineOptions.isVisible) width(mPolylineOptions.width) zIndex(mPolylineOptions.zIndex) - pattern(getPattern()) - startCap(getStartCap()) - endCap(getEndCap()) + // Read from mPolylineOptions: inside apply, getPattern() and the cap getters would + // resolve to the new PolylineOptions instead of this style. + pattern(mPolylineOptions.pattern) + startCap(mPolylineOptions.startCap) + endCap(mPolylineOptions.endCap) } override fun toString(): String = diff --git a/data/src/main/java/com/google/maps/android/data/kml/KmlLayer.kt b/data/src/main/java/com/google/maps/android/data/kml/KmlLayer.kt index 3eac2d5c3..2ef0bc1b5 100644 --- a/data/src/main/java/com/google/maps/android/data/kml/KmlLayer.kt +++ b/data/src/main/java/com/google/maps/android/data/kml/KmlLayer.kt @@ -17,6 +17,7 @@ package com.google.maps.android.data.kml import android.content.Context import android.graphics.Color +import android.util.Log import com.google.android.gms.maps.GoogleMap import com.google.android.gms.maps.model.LatLng import com.google.android.gms.maps.model.LatLngBounds @@ -37,6 +38,8 @@ import java.io.IOException import java.io.InputStream import java.nio.charset.StandardCharsets +private const val LOG_TAG = "KmlLayer" + @Deprecated("Use the new platform-agnostic data layer and renderer instead.") public class KmlLayer : Layer { private val mPlacemarks = mutableListOf() @@ -131,8 +134,16 @@ public class KmlLayer : Layer { // Parse Document kmlObj.document?.let { doc -> - val styles = doc.styles.associate { it.id!! to toLegacyStyle(it) } - val styleMaps = doc.styleMaps.associate { it.id!! to (it.pairs.firstOrNull { p -> p.key == "normal" }?.styleUrl ?: "") } + // Shared styles and style maps can only be referenced by id, so ones without an id + // are skipped rather than failing the whole layer. + val styles = doc.styles.mapNotNull { style -> style.id?.let { it to toLegacyStyle(style) } }.toMap() + val styleMaps = + doc.styleMaps + .mapNotNull { styleMap -> + styleMap.id?.let { id -> + id to (styleMap.pairs.firstOrNull { p -> p.key == "normal" }?.styleUrl ?: "") + } + }.toMap() val placemarksMap = HashMap() doc.placemarks.forEach { p -> @@ -146,7 +157,7 @@ public class KmlLayer : Layer { val groundOverlaysMap = HashMap() doc.groundOverlays.forEach { g -> - groundOverlaysMap[toLegacyGroundOverlay(g)] = null + toLegacyGroundOverlay(g)?.let { groundOverlaysMap[it] = null } } val properties = HashMap() @@ -171,7 +182,7 @@ public class KmlLayer : Layer { if (kmlObj.document == null) { kmlObj.placemark?.let { mPlacemarks.add(toLegacyPlacemark(it, emptyMap(), emptyMap())) } kmlObj.folder?.let { mContainers.add(toLegacyContainer(it, emptyMap(), emptyMap())) } - kmlObj.groundOverlay?.let { mGroundOverlays.add(toLegacyGroundOverlay(it)) } + kmlObj.groundOverlay?.let { overlay -> toLegacyGroundOverlay(overlay)?.let { mGroundOverlays.add(it) } } } } @@ -213,7 +224,9 @@ public class KmlLayer : Layer { } val styleUrl = placemark.styleUrl?.substringAfter("#") ?: "" - val resolvedStyleUrl = styleMaps[styleUrl] ?: styleUrl + // Style map entries keep the '#' of their normal styleUrl, while shared styles are keyed + // by bare id. + val resolvedStyleUrl = styleMaps[styleUrl]?.substringAfter("#") ?: styleUrl val inlineStyle = placemark.style?.let { toLegacyStyle(it) } ?: styles[resolvedStyleUrl] return KmlPlacemark(geometry, resolvedStyleUrl, inlineStyle, properties) @@ -240,7 +253,7 @@ public class KmlLayer : Layer { val groundOverlaysMap = HashMap() folder.groundOverlays.forEach { g -> - groundOverlaysMap[toLegacyGroundOverlay(g)] = null + toLegacyGroundOverlay(g)?.let { groundOverlaysMap[it] = null } } return KmlContainer( @@ -254,14 +267,23 @@ public class KmlLayer : Layer { ) } - private fun toLegacyGroundOverlay(groundOverlay: com.google.maps.android.data.parser.kml.GroundOverlay): KmlGroundOverlay { + /** + * Returns null for overlays without a LatLonBox, such as those positioned with gx:LatLonQuad, + * which a ground overlay on the map cannot represent. + */ + private fun toLegacyGroundOverlay(groundOverlay: com.google.maps.android.data.parser.kml.GroundOverlay): KmlGroundOverlay? { + val latLonBox = groundOverlay.latLonBox + if (latLonBox == null) { + Log.w(LOG_TAG, "Skipping GroundOverlay ${groundOverlay.name ?: ""} without a LatLonBox") + return null + } val properties = mutableMapOf() groundOverlay.name?.let { properties["name"] = it } val bounds = LatLngBounds( - LatLng(groundOverlay.latLonBox!!.south, groundOverlay.latLonBox.west), - LatLng(groundOverlay.latLonBox.north, groundOverlay.latLonBox.east), + LatLng(latLonBox.south, latLonBox.west), + LatLng(latLonBox.north, latLonBox.east), ) return KmlGroundOverlay( @@ -270,7 +292,7 @@ public class KmlLayer : Layer { drawOrder = groundOverlay.drawOrder?.toFloat() ?: 0f, visibility = if (groundOverlay.visibility) 1 else 0, properties = properties, - rotation = groundOverlay.latLonBox.rotation?.toFloat() ?: 0f, + rotation = latLonBox.rotation?.toFloat() ?: 0f, ) } diff --git a/data/src/main/java/com/google/maps/android/data/kml/KmlStyle.kt b/data/src/main/java/com/google/maps/android/data/kml/KmlStyle.kt index 696b95b09..73892d982 100644 --- a/data/src/main/java/com/google/maps/android/data/kml/KmlStyle.kt +++ b/data/src/main/java/com/google/maps/android/data/kml/KmlStyle.kt @@ -40,6 +40,9 @@ public class KmlStyle : Style() { private var mPolyRandomColorMode = false internal var mMarkerColor = 0f + // The ARGB color behind mMarkerColor, which is only a hue. KML's default color is white. + private var mMarkerArgb = Color.WHITE + public fun setInfoWindowText(text: String) { mBalloonOptions["text"] = text } @@ -89,6 +92,7 @@ public class KmlStyle : Style() { public fun setMarkerColor(color: String) { val integerColor = Color.parseColor("#" + convertColor(color)) + mMarkerArgb = integerColor mMarkerColor = getHueValue(integerColor) mMarkerOptions.icon(BitmapDescriptorFactory.defaultMarker(mMarkerColor)) mStylesSet.add("markerColor") @@ -149,7 +153,7 @@ public class KmlStyle : Style() { newMarkerOption.rotation(mMarkerOptions.rotation) newMarkerOption.anchor(mMarkerOptions.anchorU, mMarkerOptions.anchorV) if (mIconRandomColorMode) { - val hue = getHueValue(computeRandomColor(mMarkerColor.toInt())) + val hue = getHueValue(computeRandomColor(mMarkerArgb)) mMarkerOptions.icon(BitmapDescriptorFactory.defaultMarker(hue)) } newMarkerOption.icon(mMarkerOptions.icon) diff --git a/data/src/test/java/com/google/maps/android/data/geojson/GeoJsonStylesTest.kt b/data/src/test/java/com/google/maps/android/data/geojson/GeoJsonStylesTest.kt index 0a8b22433..805a408a1 100644 --- a/data/src/test/java/com/google/maps/android/data/geojson/GeoJsonStylesTest.kt +++ b/data/src/test/java/com/google/maps/android/data/geojson/GeoJsonStylesTest.kt @@ -23,7 +23,6 @@ import com.google.android.gms.maps.model.RoundCap import com.google.android.gms.maps.model.SquareCap import com.google.common.truth.Truth.assertThat import io.mockk.mockk -import org.junit.Ignore import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @@ -143,11 +142,9 @@ class GeoJsonStylesTest { } /** - * Inside `PolylineOptions().apply { }`, toPolylineOptions calls getPattern(), getStartCap() - * and getEndCap() unqualified, which resolve to the new PolylineOptions' own getters rather - * than the style's. The pattern and caps of a GeoJSON line style are therefore never drawn. + * Regression test: toPolylineOptions used to call getPattern() and the cap getters inside + * `PolylineOptions().apply { }`, which resolved to the new options instead of the style. */ - @Ignore("Known bug: toPolylineOptions drops the pattern and the start and end caps") @Test fun `polyline options carry the pattern and caps`() { val style = GeoJsonLineStringStyle() diff --git a/data/src/test/java/com/google/maps/android/data/kml/KmlLayerParsingTest.kt b/data/src/test/java/com/google/maps/android/data/kml/KmlLayerParsingTest.kt index 8f69b029d..f04fb37f2 100644 --- a/data/src/test/java/com/google/maps/android/data/kml/KmlLayerParsingTest.kt +++ b/data/src/test/java/com/google/maps/android/data/kml/KmlLayerParsingTest.kt @@ -22,7 +22,6 @@ import com.google.android.gms.maps.GoogleMap import com.google.android.gms.maps.model.LatLng import com.google.common.truth.Truth.assertThat import io.mockk.mockk -import org.junit.Ignore import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @@ -180,12 +179,9 @@ class KmlLayerParsingTest { } /** - * Style maps store the normal styleUrl with its leading '#' ("#red"), but shared styles are - * keyed by bare id ("red"). Resolving a placemark through a StyleMap therefore looks up - * "#red", finds nothing and leaves the placemark unstyled. Google Earth exports put a - * StyleMap on almost every placemark. + * Regression test: style maps keep the '#' of their normal styleUrl ("#red") while shared + * styles are keyed by bare id, so placemarks styled through a StyleMap used to be unstyled. */ - @Ignore("Known bug: styles referenced through a StyleMap are never applied") @Test fun `style map resolves to the normal shared style`() { val layer = layerOf( @@ -326,10 +322,8 @@ class KmlLayerParsingTest { } /** - * Shared styles are indexed with `it.id!!`. A shared Style without an id is unusual but - * valid KML, and it makes the KmlLayer constructor throw a NullPointerException. + * Regression test: a shared Style without an id used to throw a NullPointerException. */ - @Ignore("Known bug: a shared Style without an id crashes KmlLayer") @Test fun `shared style without an id is ignored`() { val layer = layerOf( @@ -347,10 +341,9 @@ class KmlLayerParsingTest { } /** - * Ground overlays read `latLonBox!!`. Overlays positioned with gx:LatLonQuad, which Google - * Earth writes for rotated overlays, have no LatLonBox and make the constructor throw. + * Regression test: overlays without a LatLonBox (gx:LatLonQuad, written by Google Earth for + * rotated overlays) used to throw a NullPointerException. They are now skipped. */ - @Ignore("Known bug: a ground overlay without a LatLonBox crashes KmlLayer") @Test fun `ground overlay without a LatLonBox does not crash the layer`() { val layer = layerOf( diff --git a/data/src/test/java/com/google/maps/android/data/kml/KmlStyleTest.kt b/data/src/test/java/com/google/maps/android/data/kml/KmlStyleTest.kt index 9c1d2f78c..37350b864 100644 --- a/data/src/test/java/com/google/maps/android/data/kml/KmlStyleTest.kt +++ b/data/src/test/java/com/google/maps/android/data/kml/KmlStyleTest.kt @@ -26,7 +26,6 @@ import io.mockk.unmockkStatic import io.mockk.verify import org.junit.After import org.junit.Before -import org.junit.Ignore import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @@ -247,12 +246,9 @@ class KmlStyleTest { } /** - * In random icon color mode, getMarkerOptions passes the marker's hue (0..360) to - * computeRandomColor, which expects an ARGB color. A green marker (hue 120) is treated as - * the color 0x00000078, so the random marker is always blue (hue 240) or black (hue 0) - * instead of a random shade of the style's green. + * Random icon color mode applies a random scale to the marker's ARGB color, so a green + * marker stays a shade of green (or black, once scaled to zero). */ - @Ignore("Known bug: random icon color mode randomizes the hue value instead of the color") @Test fun `random icon color mode keeps the marker hue`() { val style = KmlStyle()