Skip to content

Commit 1d809a4

Browse files
Seth BourgetSeth BourgetŁukasz Paczos
authored
Reinitialize route line layers every time initialize is called. (#6793)
* Modified the initialize layers behavior to always apply the current options even if it means removing the existing layers. * review updates --------- Co-authored-by: Seth Bourget <seth@cafesilencio.net> Co-authored-by: Łukasz Paczos <lukasz.paczos@mapbox.com>
1 parent 50859cd commit 1d809a4

File tree

8 files changed

+351
-56
lines changed

8 files changed

+351
-56
lines changed

‎changelog/unreleased/bugfixes/6793.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- :warning: If you're recreating the `MapboxRouteLineView` instance, for example to change the `MapboxRouteLineOptions`, make sure that your first interaction restores the state and re-applies the options by calling `MapboxRouteLineApi.getRouteDrawData` and passing the result to `MapboxRouteLineView.renderRouteDrawData`. This is a necessary change to fix an issue where `MapboxRouteLineOptions` provided in a new instance of `MapboxRouteLineView` were ignored if the style object used with `render` functions already had route line layers initialized.

‎libnavui-maps/src/main/java/com/mapbox/navigation/ui/maps/internal/route/line/MapboxRouteLineUtils.kt‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1751,4 +1751,31 @@ internal object MapboxRouteLineUtils {
17511751
.iconIgnorePlacement(true)
17521752
.iconKeepUpright(true)
17531753
}
1754+
1755+
internal fun removeLayersAndSources(style: Style) {
1756+
style.removeStyleSource(RouteLayerConstants.LAYER_GROUP_1_SOURCE_ID)
1757+
style.removeStyleSource(RouteLayerConstants.LAYER_GROUP_2_SOURCE_ID)
1758+
style.removeStyleSource(RouteLayerConstants.LAYER_GROUP_3_SOURCE_ID)
1759+
style.removeStyleSource(RouteLayerConstants.WAYPOINT_SOURCE_ID)
1760+
style.removeStyleLayer(RouteLayerConstants.TOP_LEVEL_ROUTE_LINE_LAYER_ID)
1761+
style.removeStyleLayer(RouteLayerConstants.BOTTOM_LEVEL_ROUTE_LINE_LAYER_ID)
1762+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_1_TRAIL_CASING)
1763+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_1_TRAIL)
1764+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_1_CASING)
1765+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_1_MAIN)
1766+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_1_TRAFFIC)
1767+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_1_RESTRICTED)
1768+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_2_TRAIL_CASING)
1769+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_2_TRAIL)
1770+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_2_CASING)
1771+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_2_MAIN)
1772+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_2_TRAFFIC)
1773+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_2_RESTRICTED)
1774+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_3_TRAIL_CASING)
1775+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_3_TRAIL)
1776+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_3_CASING)
1777+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_3_MAIN)
1778+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_3_TRAFFIC)
1779+
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_3_RESTRICTED)
1780+
}
17541781
}

‎libnavui-maps/src/main/java/com/mapbox/navigation/ui/maps/route/line/api/MapboxRouteLineView.kt‎

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import com.mapbox.navigation.ui.maps.internal.route.line.MapboxRouteLineUtils.ge
1818
import com.mapbox.navigation.ui.maps.internal.route.line.MapboxRouteLineUtils.layerGroup1SourceLayerIds
1919
import com.mapbox.navigation.ui.maps.internal.route.line.MapboxRouteLineUtils.layerGroup2SourceLayerIds
2020
import com.mapbox.navigation.ui.maps.internal.route.line.MapboxRouteLineUtils.layerGroup3SourceLayerIds
21+
import com.mapbox.navigation.ui.maps.internal.route.line.MapboxRouteLineUtils.layersAreInitialized
2122
import com.mapbox.navigation.ui.maps.internal.route.line.MapboxRouteLineUtils.sourceLayerMap
2223
import com.mapbox.navigation.ui.maps.route.RouteLayerConstants
2324
import com.mapbox.navigation.ui.maps.route.RouteLayerConstants.LAYER_GROUP_1_CASING
@@ -73,11 +74,17 @@ import org.jetbrains.annotations.TestOnly
7374
* Many of the method calls execute tasks on a background thread. A cancel method is provided
7475
* in this class which will cancel the background tasks.
7576
*
77+
* If you're recreating the [MapboxRouteLineView] instance, for example to change the
78+
* [MapboxRouteLineOptions], make sure that your first interaction restores the state and re-applies
79+
* the options by calling [MapboxRouteLineApi.getRouteDrawData] and passing the result to [MapboxRouteLineView.renderRouteDrawData].
80+
*
7681
* @param options resource options used rendering the route line on the map
7782
*/
7883
@UiThread
7984
class MapboxRouteLineView(options: MapboxRouteLineOptions) {
8085

86+
private var rebuildLayersOnFirstRender: Boolean = true
87+
8188
private companion object {
8289
private const val TAG = "MbxRouteLineView"
8390
}
@@ -88,7 +95,8 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
8895
var options: MapboxRouteLineOptions = options
8996
@Deprecated(
9097
message = "Avoid using this setter as it will not correctly " +
91-
"re-apply all mutated parameters."
98+
"re-apply all mutated parameters. " +
99+
"Recreate the instance and provide options via constructor instead."
92100
)
93101
set
94102

@@ -149,7 +157,7 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
149157
* @param style a valid [Style] instance
150158
*/
151159
fun initializeLayers(style: Style) {
152-
MapboxRouteLineUtils.initializeLayers(style, options)
160+
rebuildSourcesAndLayersIfNeeded(style)
153161
}
154162

155163
/**
@@ -159,7 +167,7 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
159167
* @param routeDrawData a [Expected<RouteLineError, RouteSetValue>]
160168
*/
161169
fun renderRouteDrawData(style: Style, routeDrawData: Expected<RouteLineError, RouteSetValue>) {
162-
MapboxRouteLineUtils.initializeLayers(style, options)
170+
rebuildSourcesAndLayersIfNeeded(style)
163171
jobControl.scope.launch(Dispatchers.Main) {
164172
mutex.withLock {
165173
val primaryRouteTrafficVisibility = getTrafficVisibility(style)
@@ -234,7 +242,9 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
234242
sourceLayerMap
235243
)
236244
}
237-
} else { {} }
245+
} else {
246+
{}
247+
}
238248
listOf(layerMoveCommand).plus(trimOffsetCommands).plus(gradientCommands)
239249
} ?: listOf()
240250
}
@@ -332,7 +342,7 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
332342
style: Style,
333343
clearRouteLineValue: Expected<RouteLineError, RouteLineClearValue>
334344
) {
335-
MapboxRouteLineUtils.initializeLayers(style, options)
345+
rebuildSourcesAndLayersIfNeeded(style)
336346
jobControl.scope.launch(Dispatchers.Main) {
337347
mutex.withLock {
338348
clearRouteLineValue.onValue { value ->
@@ -837,7 +847,8 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
837847
}
838848
else -> null
839849
}
840-
} else -> {
850+
}
851+
else -> {
841852
when (it) {
842853
in casingLayerIds -> {
843854
options.resourceProvider.alternativeRouteCasingLineScaleExpression
@@ -862,4 +873,33 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
862873
}
863874
}
864875
}
876+
877+
private fun rebuildSourcesAndLayersIfNeeded(style: Style) {
878+
if (rebuildLayersOnFirstRender || !layersAreInitialized(style, options)) {
879+
rebuildLayersOnFirstRender = false
880+
resetLayers(style)
881+
}
882+
}
883+
884+
private fun resetLayers(style: Style) {
885+
sourceToFeatureMap.clear()
886+
sourceToFeatureMap[MapboxRouteLineUtils.layerGroup1SourceKey] = RouteLineFeatureId(null)
887+
sourceToFeatureMap[MapboxRouteLineUtils.layerGroup2SourceKey] = RouteLineFeatureId(null)
888+
sourceToFeatureMap[MapboxRouteLineUtils.layerGroup3SourceKey] = RouteLineFeatureId(null)
889+
primaryRouteLineLayerGroup = setOf()
890+
listOf(
891+
RouteLayerConstants.LAYER_GROUP_1_SOURCE_ID,
892+
RouteLayerConstants.LAYER_GROUP_2_SOURCE_ID,
893+
RouteLayerConstants.LAYER_GROUP_3_SOURCE_ID,
894+
RouteLayerConstants.WAYPOINT_SOURCE_ID
895+
).forEach {
896+
updateSource(
897+
style,
898+
it,
899+
FeatureCollection.fromFeatures(listOf())
900+
)
901+
}
902+
MapboxRouteLineUtils.removeLayersAndSources(style)
903+
MapboxRouteLineUtils.initializeLayers(style, options)
904+
}
865905
}

‎libnavui-maps/src/test/java/com/mapbox/navigation/ui/maps/internal/route/line/MapboxRouteLineUtilsTest.kt‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2028,6 +2028,41 @@ class MapboxRouteLineUtilsTest {
20282028
assertFalse(result)
20292029
}
20302030

2031+
@Test
2032+
fun removeLayersAndSources() {
2033+
val style = mockk<Style> {
2034+
every { removeStyleLayer(any()) } returns ExpectedFactory.createNone()
2035+
every { removeStyleSource(any()) } returns ExpectedFactory.createNone()
2036+
}
2037+
2038+
MapboxRouteLineUtils.removeLayersAndSources(style)
2039+
2040+
verify { style.removeStyleSource(LAYER_GROUP_1_SOURCE_ID) }
2041+
verify { style.removeStyleSource(LAYER_GROUP_2_SOURCE_ID) }
2042+
verify { style.removeStyleSource(LAYER_GROUP_3_SOURCE_ID) }
2043+
verify { style.removeStyleSource(WAYPOINT_SOURCE_ID) }
2044+
verify { style.removeStyleLayer(TOP_LEVEL_ROUTE_LINE_LAYER_ID) }
2045+
verify { style.removeStyleLayer(BOTTOM_LEVEL_ROUTE_LINE_LAYER_ID) }
2046+
verify { style.removeStyleLayer(LAYER_GROUP_1_TRAIL_CASING) }
2047+
verify { style.removeStyleLayer(LAYER_GROUP_1_TRAIL) }
2048+
verify { style.removeStyleLayer(LAYER_GROUP_1_CASING) }
2049+
verify { style.removeStyleLayer(LAYER_GROUP_1_MAIN) }
2050+
verify { style.removeStyleLayer(LAYER_GROUP_1_TRAFFIC) }
2051+
verify { style.removeStyleLayer(LAYER_GROUP_1_RESTRICTED) }
2052+
verify { style.removeStyleLayer(LAYER_GROUP_2_TRAIL_CASING) }
2053+
verify { style.removeStyleLayer(LAYER_GROUP_2_TRAIL) }
2054+
verify { style.removeStyleLayer(LAYER_GROUP_2_CASING) }
2055+
verify { style.removeStyleLayer(LAYER_GROUP_2_MAIN) }
2056+
verify { style.removeStyleLayer(LAYER_GROUP_2_TRAFFIC) }
2057+
verify { style.removeStyleLayer(LAYER_GROUP_2_RESTRICTED) }
2058+
verify { style.removeStyleLayer(LAYER_GROUP_3_TRAIL_CASING) }
2059+
verify { style.removeStyleLayer(LAYER_GROUP_3_TRAIL) }
2060+
verify { style.removeStyleLayer(LAYER_GROUP_3_CASING) }
2061+
verify { style.removeStyleLayer(LAYER_GROUP_3_MAIN) }
2062+
verify { style.removeStyleLayer(LAYER_GROUP_3_TRAFFIC) }
2063+
verify { style.removeStyleLayer(LAYER_GROUP_3_RESTRICTED) }
2064+
}
2065+
20312066
private fun <T> listElementsAreEqual(
20322067
first: List<T>,
20332068
second: List<T>,

‎libnavui-maps/src/test/java/com/mapbox/navigation/ui/maps/route/line/api/MapboxRouteLineViewTest.kt‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,8 @@ class MapboxRouteLineViewTest {
149149
every {
150150
styleLayerExists(BOTTOM_LEVEL_ROUTE_LINE_LAYER_ID)
151151
} returns true
152+
every { removeStyleSource(any()) } returns ExpectedFactory.createNone()
153+
every { removeStyleLayer(any()) } returns ExpectedFactory.createNone()
152154
}
153155
}
154156

@@ -169,6 +171,88 @@ class MapboxRouteLineViewTest {
169171
every {
170172
setStyleSourceProperty(WAYPOINT_SOURCE_ID, any(), any())
171173
} returns ExpectedFactory.createNone()
174+
every { getSource(LAYER_GROUP_1_SOURCE_ID) } returns null
175+
every { getSource(LAYER_GROUP_2_SOURCE_ID) } returns null
176+
every { getSource(LAYER_GROUP_3_SOURCE_ID) } returns null
177+
every { getSource(WAYPOINT_SOURCE_ID) } returns null
178+
every {
179+
removeStyleSource(LAYER_GROUP_1_SOURCE_ID)
180+
} returns ExpectedFactory.createNone()
181+
every {
182+
removeStyleSource(LAYER_GROUP_2_SOURCE_ID)
183+
} returns ExpectedFactory.createNone()
184+
every {
185+
removeStyleSource(LAYER_GROUP_3_SOURCE_ID)
186+
} returns ExpectedFactory.createNone()
187+
every {
188+
removeStyleSource(WAYPOINT_SOURCE_ID)
189+
} returns ExpectedFactory.createNone()
190+
every {
191+
removeStyleSource(TOP_LEVEL_ROUTE_LINE_LAYER_ID)
192+
} returns ExpectedFactory.createNone()
193+
every {
194+
removeStyleSource(BOTTOM_LEVEL_ROUTE_LINE_LAYER_ID)
195+
} returns ExpectedFactory.createNone()
196+
every {
197+
removeStyleLayer(TOP_LEVEL_ROUTE_LINE_LAYER_ID)
198+
} returns ExpectedFactory.createNone()
199+
every {
200+
removeStyleLayer(BOTTOM_LEVEL_ROUTE_LINE_LAYER_ID)
201+
} returns ExpectedFactory.createNone()
202+
every {
203+
removeStyleLayer(LAYER_GROUP_1_TRAIL_CASING)
204+
} returns ExpectedFactory.createNone()
205+
every {
206+
removeStyleLayer(LAYER_GROUP_1_TRAIL)
207+
} returns ExpectedFactory.createNone()
208+
every {
209+
removeStyleLayer(LAYER_GROUP_1_CASING)
210+
} returns ExpectedFactory.createNone()
211+
every {
212+
removeStyleLayer(LAYER_GROUP_1_MAIN)
213+
} returns ExpectedFactory.createNone()
214+
every {
215+
removeStyleLayer(LAYER_GROUP_1_TRAFFIC)
216+
} returns ExpectedFactory.createNone()
217+
every {
218+
removeStyleLayer(LAYER_GROUP_1_RESTRICTED)
219+
} returns ExpectedFactory.createNone()
220+
every {
221+
removeStyleLayer(LAYER_GROUP_2_TRAIL_CASING)
222+
} returns ExpectedFactory.createNone()
223+
every {
224+
removeStyleLayer(LAYER_GROUP_2_TRAIL)
225+
} returns ExpectedFactory.createNone()
226+
every {
227+
removeStyleLayer(LAYER_GROUP_2_CASING)
228+
} returns ExpectedFactory.createNone()
229+
every {
230+
removeStyleLayer(LAYER_GROUP_2_MAIN)
231+
} returns ExpectedFactory.createNone()
232+
every {
233+
removeStyleLayer(LAYER_GROUP_2_TRAFFIC)
234+
} returns ExpectedFactory.createNone()
235+
every {
236+
removeStyleLayer(LAYER_GROUP_2_RESTRICTED)
237+
} returns ExpectedFactory.createNone()
238+
every {
239+
removeStyleLayer(LAYER_GROUP_3_TRAIL_CASING)
240+
} returns ExpectedFactory.createNone()
241+
every {
242+
removeStyleLayer(LAYER_GROUP_3_TRAIL)
243+
} returns ExpectedFactory.createNone()
244+
every {
245+
removeStyleLayer(LAYER_GROUP_3_CASING)
246+
} returns ExpectedFactory.createNone()
247+
every {
248+
removeStyleLayer(LAYER_GROUP_3_MAIN)
249+
} returns ExpectedFactory.createNone()
250+
every {
251+
removeStyleLayer(LAYER_GROUP_3_TRAFFIC)
252+
} returns ExpectedFactory.createNone()
253+
every {
254+
removeStyleLayer(LAYER_GROUP_3_RESTRICTED)
255+
} returns ExpectedFactory.createNone()
172256
}.also {
173257
mockCheckForLayerInitialization(it)
174258
}
@@ -242,6 +326,66 @@ class MapboxRouteLineViewTest {
242326
unmockkStatic("com.mapbox.maps.extension.style.sources.SourceUtils")
243327
}
244328

329+
@Test
330+
fun renderClearRouteLineValue_noInitializeRepeat() = coroutineRule.runBlockingTest {
331+
mockkStatic("com.mapbox.maps.extension.style.sources.SourceUtils")
332+
mockkObject(MapboxRouteLineUtils)
333+
val options = MapboxRouteLineOptions.Builder(ctx).build()
334+
val primaryRouteFeatureCollection =
335+
FeatureCollection.fromFeatures(listOf(getEmptyFeature(UUID.randomUUID().toString())))
336+
val altRoutesFeatureCollection =
337+
FeatureCollection.fromFeatures(listOf(getEmptyFeature(UUID.randomUUID().toString())))
338+
val waypointsFeatureCollection =
339+
FeatureCollection.fromFeatures(listOf(getEmptyFeature(UUID.randomUUID().toString())))
340+
val primaryRouteSource = mockk<GeoJsonSource>(relaxed = true)
341+
val altRoute1Source = mockk<GeoJsonSource>(relaxed = true)
342+
val altRoute2Source = mockk<GeoJsonSource>(relaxed = true)
343+
val wayPointSource = mockk<GeoJsonSource>(relaxed = true)
344+
val topLevelRouteLayer = StyleObjectInfo(
345+
TOP_LEVEL_ROUTE_LINE_LAYER_ID,
346+
"background"
347+
)
348+
val bottomLevelRouteLayer = StyleObjectInfo(
349+
BOTTOM_LEVEL_ROUTE_LINE_LAYER_ID,
350+
"background"
351+
)
352+
val mainLayer = StyleObjectInfo(
353+
LAYER_GROUP_1_MAIN,
354+
"line"
355+
)
356+
val style = mockk<Style> {
357+
every { getSource(LAYER_GROUP_1_SOURCE_ID) } returns primaryRouteSource
358+
every { getSource(LAYER_GROUP_2_SOURCE_ID) } returns altRoute1Source
359+
every { getSource(LAYER_GROUP_3_SOURCE_ID) } returns altRoute2Source
360+
every { getSource(WAYPOINT_SOURCE_ID) } returns wayPointSource
361+
every { styleLayers } returns listOf(
362+
bottomLevelRouteLayer,
363+
mainLayer,
364+
topLevelRouteLayer
365+
)
366+
}.also {
367+
mockCheckForLayerInitialization(it)
368+
}
369+
370+
val state: Expected<RouteLineError, RouteLineClearValue> = ExpectedFactory.createValue(
371+
RouteLineClearValue(
372+
primaryRouteFeatureCollection,
373+
listOf(altRoutesFeatureCollection, altRoutesFeatureCollection),
374+
waypointsFeatureCollection
375+
)
376+
)
377+
378+
pauseDispatcher {
379+
val view = MapboxRouteLineView(options)
380+
view.renderClearRouteLineValue(style, state)
381+
view.renderClearRouteLineValue(style, state)
382+
}
383+
384+
verify(exactly = 1) { MapboxRouteLineUtils.initializeLayers(style, options) }
385+
unmockkObject(MapboxRouteLineUtils)
386+
unmockkStatic("com.mapbox.maps.extension.style.sources.SourceUtils")
387+
}
388+
245389
@Test
246390
fun renderTraveledRouteLineUpdate() = coroutineRule.runBlockingTest {
247391
mockkStatic("com.mapbox.maps.extension.style.layers.LayerUtils")
@@ -1095,6 +1239,7 @@ class MapboxRouteLineViewTest {
10951239
view.renderRouteDrawData(style, state)
10961240
view.renderRouteDrawData(style2, state2)
10971241

1242+
verify(exactly = 1) { MapboxRouteLineUtils.initializeLayers(style, options) }
10981243
verify(exactly = 1) { style.moveStyleLayer(LAYER_GROUP_1_TRAIL_CASING, any()) }
10991244
verify(exactly = 1) { style.moveStyleLayer(LAYER_GROUP_1_TRAIL, any()) }
11001245
verify(exactly = 1) { style.moveStyleLayer(LAYER_GROUP_1_CASING, any()) }

0 commit comments

Comments
 (0)