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