From a4da67f8ba156234532d8940a8b0e7c9be4d8434 Mon Sep 17 00:00:00 2001 From: yunik Date: Mon, 3 Oct 2022 15:18:39 +0300 Subject: [PATCH] Fix possible crash when terrain source is added multiple times using runtime Style DSL. --- .../java/com/mapbox/maps/StyleLoadTest.kt | 50 +++++++++++++++++++ .../main/java/com/mapbox/maps/MapboxMap.kt | 2 +- .../java/com/mapbox/maps/StyleObserver.kt | 21 ++++++-- 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/sdk/src/androidTest/java/com/mapbox/maps/StyleLoadTest.kt b/sdk/src/androidTest/java/com/mapbox/maps/StyleLoadTest.kt index 4189d73213..c6f0bdf584 100644 --- a/sdk/src/androidTest/java/com/mapbox/maps/StyleLoadTest.kt +++ b/sdk/src/androidTest/java/com/mapbox/maps/StyleLoadTest.kt @@ -4,7 +4,10 @@ import androidx.test.annotation.UiThreadTest import androidx.test.ext.junit.rules.ActivityScenarioRule import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.LargeTest +import com.mapbox.maps.extension.style.expressions.dsl.generated.literal +import com.mapbox.maps.extension.style.sources.generated.rasterDemSource import com.mapbox.maps.extension.style.style +import com.mapbox.maps.extension.style.terrain.generated.terrain import junit.framework.TestCase.* import org.junit.* import org.junit.runner.RunWith @@ -169,6 +172,53 @@ class StyleLoadTest { assertEquals(loadedStyles, 1) } + @Test + fun testTerrainDoesNotCrashOnMultipleStyleLoad() { + fun loadStyleWithTerrain( + uri: String = Style.DARK, + terrainSource: String = "TERRAIN_SOURCE", + demSourceUri : String= "mapbox://mapbox.mapbox-terrain-dem-v1" + ) { + mapboxMap.loadStyle( + styleExtension = style(uri) { + // we need terrain for the drape-mipmap parameter to be effective to reduce flickering + +rasterDemSource(terrainSource) { + url(demSourceUri) + // 514 specifies padded DEM tile and provides better performance than 512 tiles. + tileSize(514) + maxzoom(3) + } + +terrain(terrainSource) { + exaggeration(literal(0)) + } + } + ) + } + + countDownLatch = CountDownLatch(1) + rule.scenario.onActivity { + it.runOnUiThread { + for (i in 0..100) { + loadStyleWithTerrain() + } + + var i = 0 + val runnable = object : Runnable { + override fun run() { + if (i++ < 100) { + loadStyleWithTerrain() + mapView.postDelayed(this, 1) + } + } + } + mapView.post(runnable) + + mapView.onStart() + } + } + countDownLatch.throwExceptionOnTimeoutMs() + } + @After @UiThreadTest fun teardown() { diff --git a/sdk/src/main/java/com/mapbox/maps/MapboxMap.kt b/sdk/src/main/java/com/mapbox/maps/MapboxMap.kt index 6f3550a953..bf17555c05 100644 --- a/sdk/src/main/java/com/mapbox/maps/MapboxMap.kt +++ b/sdk/src/main/java/com/mapbox/maps/MapboxMap.kt @@ -326,7 +326,7 @@ class MapboxMap : private fun initializeStyleLoad( onStyleLoaded: Style.OnStyleLoaded? = null, - styleDataStyleLoadedListener: Style.OnStyleLoaded? = null, + styleDataStyleLoadedListener: Style.OnStyleLoaded, styleDataSpritesLoadedListener: Style.OnStyleLoaded? = null, styleDataSourcesLoadedListener: Style.OnStyleLoaded? = null, onMapLoadErrorListener: OnMapLoadErrorListener? = null, diff --git a/sdk/src/main/java/com/mapbox/maps/StyleObserver.kt b/sdk/src/main/java/com/mapbox/maps/StyleObserver.kt index 53293820e5..d4c87bea3e 100644 --- a/sdk/src/main/java/com/mapbox/maps/StyleObserver.kt +++ b/sdk/src/main/java/com/mapbox/maps/StyleObserver.kt @@ -22,8 +22,15 @@ internal class StyleObserver( private var userStyleLoadedListener: Style.OnStyleLoaded? = null + // fired when [StyleDataType.STYLE] arrives, first of all style data loaded events private var styleDataStyleLoadedListener: Style.OnStyleLoaded? = null + // it the listener is not null - style load has been started + // but the initial event has NOT arrived yet + private val isWaitingStyleDataStyleEvent + get() = styleDataStyleLoadedListener == null + // fired when [StyleDataType.SPRITE] arrives private var styleDataSpritesLoadedListener: Style.OnStyleLoaded? = null + // fired when [StyleDataType.SOURCES] arrives private var styleDataSourcesLoadedListener: Style.OnStyleLoaded? = null private var loadStyleErrorListener: OnMapLoadErrorListener? = null @@ -50,7 +57,7 @@ internal class StyleObserver( */ fun setLoadStyleListener( userOnStyleLoaded: Style.OnStyleLoaded?, - styleDataStyleLoadedListener: Style.OnStyleLoaded? = null, + styleDataStyleLoadedListener: Style.OnStyleLoaded, styleDataSpritesLoadedListener: Style.OnStyleLoaded? = null, styleDataSourcesLoadedListener: Style.OnStyleLoaded? = null, onMapLoadErrorListener: OnMapLoadErrorListener? @@ -115,10 +122,18 @@ internal class StyleObserver( styleDataStyleLoadedListener = null } StyleDataType.SPRITE -> { - onStyleSpritesReady() + // if we're waiting for the [StyleDataType.STYLE] event - then this [StyleDataType.SPRITE] + // is related to the previously loaded style, don't notify style extension DSL about it + if (!isWaitingStyleDataStyleEvent) { + onStyleSpritesReady() + } } StyleDataType.SOURCES -> { - onStyleSourcesReady() + // if we're waiting for the [StyleDataType.STYLE] event - then this [StyleDataType.SOURCES] + // is related to the previously loaded style, don't notify style extension DSL about it + if (!isWaitingStyleDataStyleEvent) { + onStyleSourcesReady() + } } } }