fix(data): apply StyleMap styles and fix four KML and GeoJSON style bugs

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 =

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<KmlPlacemark>()
@@ -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<KmlPlacemark, Any?>()
             doc.placemarks.forEach { p ->
@@ -146,7 +157,7 @@ public class KmlLayer : Layer {
 
             val groundOverlaysMap = HashMap<KmlGroundOverlay, com.google.android.gms.maps.model.GroundOverlay?>()
             doc.groundOverlays.forEach { g ->
-                groundOverlaysMap[toLegacyGroundOverlay(g)] = null
+                toLegacyGroundOverlay(g)?.let { groundOverlaysMap[it] = null }
             }
 
             val properties = HashMap<String, String>()
@@ -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<KmlGroundOverlay, com.google.android.gms.maps.model.GroundOverlay?>()
         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<String, String>()
         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,
         )
     }
 

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)

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()

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(

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()