Skip to content

Reinitialize route line layers every time initialize is called. #6793

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
LukasPaczos merged 2 commits into from
Feb 3, 2023
Merged

Reinitialize route line layers every time initialize is called. #6793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d6b3815
Modified the initialize layers behavior to always apply the current o…
Jan 5, 2023
9905db4
review updates
Feb 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/unreleased/bugfixes/6793.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +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.
27 changes: 27 additions & 0 deletions ...s/src/main/java/com/mapbox/navigation/ui/maps/internal/route/line/MapboxRouteLineUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1751,4 +1751,31 @@ internal object MapboxRouteLineUtils {
.iconIgnorePlacement(true)
.iconKeepUpright(true)
}

internal fun removeLayersAndSources(style: Style) {
style.removeStyleSource(RouteLayerConstants.LAYER_GROUP_1_SOURCE_ID)
style.removeStyleSource(RouteLayerConstants.LAYER_GROUP_2_SOURCE_ID)
style.removeStyleSource(RouteLayerConstants.LAYER_GROUP_3_SOURCE_ID)
style.removeStyleSource(RouteLayerConstants.WAYPOINT_SOURCE_ID)
style.removeStyleLayer(RouteLayerConstants.TOP_LEVEL_ROUTE_LINE_LAYER_ID)
style.removeStyleLayer(RouteLayerConstants.BOTTOM_LEVEL_ROUTE_LINE_LAYER_ID)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_1_TRAIL_CASING)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_1_TRAIL)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_1_CASING)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_1_MAIN)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_1_TRAFFIC)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_1_RESTRICTED)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_2_TRAIL_CASING)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_2_TRAIL)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_2_CASING)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_2_MAIN)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_2_TRAFFIC)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_2_RESTRICTED)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_3_TRAIL_CASING)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_3_TRAIL)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_3_CASING)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_3_MAIN)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_3_TRAFFIC)
style.removeStyleLayer(RouteLayerConstants.LAYER_GROUP_3_RESTRICTED)
}
}
52 changes: 46 additions & 6 deletions ...ui-maps/src/main/java/com/mapbox/navigation/ui/maps/route/line/api/MapboxRouteLineView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import com.mapbox.navigation.ui.maps.internal.route.line.MapboxRouteLineUtils.ge
import com.mapbox.navigation.ui.maps.internal.route.line.MapboxRouteLineUtils.layerGroup1SourceLayerIds
import com.mapbox.navigation.ui.maps.internal.route.line.MapboxRouteLineUtils.layerGroup2SourceLayerIds
import com.mapbox.navigation.ui.maps.internal.route.line.MapboxRouteLineUtils.layerGroup3SourceLayerIds
import com.mapbox.navigation.ui.maps.internal.route.line.MapboxRouteLineUtils.layersAreInitialized
import com.mapbox.navigation.ui.maps.internal.route.line.MapboxRouteLineUtils.sourceLayerMap
import com.mapbox.navigation.ui.maps.route.RouteLayerConstants
import com.mapbox.navigation.ui.maps.route.RouteLayerConstants.LAYER_GROUP_1_CASING
Expand Down Expand Up @@ -73,11 +74,17 @@ import org.jetbrains.annotations.TestOnly
* Many of the method calls execute tasks on a background thread. A cancel method is provided
* in this class which will cancel the background tasks.
*
* 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].
*
* @param options resource options used rendering the route line on the map
*/
@UiThread
class MapboxRouteLineView(options: MapboxRouteLineOptions) {

private var rebuildLayersOnFirstRender: Boolean = true

private companion object {
private const val TAG = "MbxRouteLineView"
}
Expand All @@ -88,7 +95,8 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
var options: MapboxRouteLineOptions = options
@Deprecated(
message = "Avoid using this setter as it will not correctly " +
"re-apply all mutated parameters."
"re-apply all mutated parameters. " +
"Recreate the instance and provide options via constructor instead."
)
set

Expand Down Expand Up @@ -149,7 +157,7 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
* @param style a valid [Style] instance
*/
fun initializeLayers(style: Style) {
MapboxRouteLineUtils.initializeLayers(style, options)
rebuildSourcesAndLayersIfNeeded(style)
}

/**
Expand All @@ -159,7 +167,7 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
* @param routeDrawData a [Expected<RouteLineError, RouteSetValue>]
*/
fun renderRouteDrawData(style: Style, routeDrawData: Expected<RouteLineError, RouteSetValue>) {
MapboxRouteLineUtils.initializeLayers(style, options)
rebuildSourcesAndLayersIfNeeded(style)
jobControl.scope.launch(Dispatchers.Main) {
mutex.withLock {
val primaryRouteTrafficVisibility = getTrafficVisibility(style)
Expand Down Expand Up @@ -234,7 +242,9 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
sourceLayerMap
)
}
} else { {} }
} else {
{}
}
listOf(layerMoveCommand).plus(trimOffsetCommands).plus(gradientCommands)
} ?: listOf()
}
Expand Down Expand Up @@ -332,7 +342,7 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
style: Style,
clearRouteLineValue: Expected<RouteLineError, RouteLineClearValue>
) {
MapboxRouteLineUtils.initializeLayers(style, options)
rebuildSourcesAndLayersIfNeeded(style)
jobControl.scope.launch(Dispatchers.Main) {
mutex.withLock {
clearRouteLineValue.onValue { value ->
Expand Down Expand Up @@ -837,7 +847,8 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
}
else -> null
}
} else -> {
}
else -> {
when (it) {
in casingLayerIds -> {
options.resourceProvider.alternativeRouteCasingLineScaleExpression
Expand All @@ -862,4 +873,33 @@ class MapboxRouteLineView(options: MapboxRouteLineOptions) {
}
}
}

private fun rebuildSourcesAndLayersIfNeeded(style: Style) {
if (rebuildLayersOnFirstRender || !layersAreInitialized(style, options)) {
rebuildLayersOnFirstRender = false
resetLayers(style)
}
}

private fun resetLayers(style: Style) {
sourceToFeatureMap.clear()
sourceToFeatureMap[MapboxRouteLineUtils.layerGroup1SourceKey] = RouteLineFeatureId(null)
sourceToFeatureMap[MapboxRouteLineUtils.layerGroup2SourceKey] = RouteLineFeatureId(null)
sourceToFeatureMap[MapboxRouteLineUtils.layerGroup3SourceKey] = RouteLineFeatureId(null)
primaryRouteLineLayerGroup = setOf()
listOf(
RouteLayerConstants.LAYER_GROUP_1_SOURCE_ID,
RouteLayerConstants.LAYER_GROUP_2_SOURCE_ID,
RouteLayerConstants.LAYER_GROUP_3_SOURCE_ID,
RouteLayerConstants.WAYPOINT_SOURCE_ID
).forEach {
updateSource(
style,
it,
FeatureCollection.fromFeatures(listOf())
)
}
MapboxRouteLineUtils.removeLayersAndSources(style)
MapboxRouteLineUtils.initializeLayers(style, options)
}
}
35 changes: 35 additions & 0 deletions ...c/test/java/com/mapbox/navigation/ui/maps/internal/route/line/MapboxRouteLineUtilsTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2028,6 +2028,41 @@ class MapboxRouteLineUtilsTest {
assertFalse(result)
}

@Test
fun removeLayersAndSources() {
val style = mockk<Style> {
every { removeStyleLayer(any()) } returns ExpectedFactory.createNone()
every { removeStyleSource(any()) } returns ExpectedFactory.createNone()
}

MapboxRouteLineUtils.removeLayersAndSources(style)

verify { style.removeStyleSource(LAYER_GROUP_1_SOURCE_ID) }
verify { style.removeStyleSource(LAYER_GROUP_2_SOURCE_ID) }
verify { style.removeStyleSource(LAYER_GROUP_3_SOURCE_ID) }
verify { style.removeStyleSource(WAYPOINT_SOURCE_ID) }
verify { style.removeStyleLayer(TOP_LEVEL_ROUTE_LINE_LAYER_ID) }
verify { style.removeStyleLayer(BOTTOM_LEVEL_ROUTE_LINE_LAYER_ID) }
verify { style.removeStyleLayer(LAYER_GROUP_1_TRAIL_CASING) }
verify { style.removeStyleLayer(LAYER_GROUP_1_TRAIL) }
verify { style.removeStyleLayer(LAYER_GROUP_1_CASING) }
verify { style.removeStyleLayer(LAYER_GROUP_1_MAIN) }
verify { style.removeStyleLayer(LAYER_GROUP_1_TRAFFIC) }
verify { style.removeStyleLayer(LAYER_GROUP_1_RESTRICTED) }
verify { style.removeStyleLayer(LAYER_GROUP_2_TRAIL_CASING) }
verify { style.removeStyleLayer(LAYER_GROUP_2_TRAIL) }
verify { style.removeStyleLayer(LAYER_GROUP_2_CASING) }
verify { style.removeStyleLayer(LAYER_GROUP_2_MAIN) }
verify { style.removeStyleLayer(LAYER_GROUP_2_TRAFFIC) }
verify { style.removeStyleLayer(LAYER_GROUP_2_RESTRICTED) }
verify { style.removeStyleLayer(LAYER_GROUP_3_TRAIL_CASING) }
verify { style.removeStyleLayer(LAYER_GROUP_3_TRAIL) }
verify { style.removeStyleLayer(LAYER_GROUP_3_CASING) }
verify { style.removeStyleLayer(LAYER_GROUP_3_MAIN) }
verify { style.removeStyleLayer(LAYER_GROUP_3_TRAFFIC) }
verify { style.removeStyleLayer(LAYER_GROUP_3_RESTRICTED) }
}

private fun <T> listElementsAreEqual(
first: List<T>,
second: List<T>,
Expand Down
145 changes: 145 additions & 0 deletions ...aps/src/test/java/com/mapbox/navigation/ui/maps/route/line/api/MapboxRouteLineViewTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ class MapboxRouteLineViewTest {
every {
styleLayerExists(BOTTOM_LEVEL_ROUTE_LINE_LAYER_ID)
} returns true
every { removeStyleSource(any()) } returns ExpectedFactory.createNone()
every { removeStyleLayer(any()) } returns ExpectedFactory.createNone()
}
}

Expand All @@ -169,6 +171,88 @@ class MapboxRouteLineViewTest {
every {
setStyleSourceProperty(WAYPOINT_SOURCE_ID, any(), any())
} returns ExpectedFactory.createNone()
every { getSource(LAYER_GROUP_1_SOURCE_ID) } returns null
every { getSource(LAYER_GROUP_2_SOURCE_ID) } returns null
every { getSource(LAYER_GROUP_3_SOURCE_ID) } returns null
every { getSource(WAYPOINT_SOURCE_ID) } returns null
every {
removeStyleSource(LAYER_GROUP_1_SOURCE_ID)
} returns ExpectedFactory.createNone()
every {
removeStyleSource(LAYER_GROUP_2_SOURCE_ID)
} returns ExpectedFactory.createNone()
every {
removeStyleSource(LAYER_GROUP_3_SOURCE_ID)
} returns ExpectedFactory.createNone()
every {
removeStyleSource(WAYPOINT_SOURCE_ID)
} returns ExpectedFactory.createNone()
every {
removeStyleSource(TOP_LEVEL_ROUTE_LINE_LAYER_ID)
} returns ExpectedFactory.createNone()
every {
removeStyleSource(BOTTOM_LEVEL_ROUTE_LINE_LAYER_ID)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(TOP_LEVEL_ROUTE_LINE_LAYER_ID)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(BOTTOM_LEVEL_ROUTE_LINE_LAYER_ID)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_1_TRAIL_CASING)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_1_TRAIL)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_1_CASING)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_1_MAIN)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_1_TRAFFIC)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_1_RESTRICTED)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_2_TRAIL_CASING)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_2_TRAIL)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_2_CASING)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_2_MAIN)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_2_TRAFFIC)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_2_RESTRICTED)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_3_TRAIL_CASING)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_3_TRAIL)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_3_CASING)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_3_MAIN)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_3_TRAFFIC)
} returns ExpectedFactory.createNone()
every {
removeStyleLayer(LAYER_GROUP_3_RESTRICTED)
} returns ExpectedFactory.createNone()
}.also {
mockCheckForLayerInitialization(it)
}
Expand Down Expand Up @@ -242,6 +326,66 @@ class MapboxRouteLineViewTest {
unmockkStatic("com.mapbox.maps.extension.style.sources.SourceUtils")
}

@Test
fun renderClearRouteLineValue_noInitializeRepeat() = coroutineRule.runBlockingTest {
mockkStatic("com.mapbox.maps.extension.style.sources.SourceUtils")
mockkObject(MapboxRouteLineUtils)
val options = MapboxRouteLineOptions.Builder(ctx).build()
val primaryRouteFeatureCollection =
FeatureCollection.fromFeatures(listOf(getEmptyFeature(UUID.randomUUID().toString())))
val altRoutesFeatureCollection =
FeatureCollection.fromFeatures(listOf(getEmptyFeature(UUID.randomUUID().toString())))
val waypointsFeatureCollection =
FeatureCollection.fromFeatures(listOf(getEmptyFeature(UUID.randomUUID().toString())))
val primaryRouteSource = mockk<GeoJsonSource>(relaxed = true)
val altRoute1Source = mockk<GeoJsonSource>(relaxed = true)
val altRoute2Source = mockk<GeoJsonSource>(relaxed = true)
val wayPointSource = mockk<GeoJsonSource>(relaxed = true)
val topLevelRouteLayer = StyleObjectInfo(
TOP_LEVEL_ROUTE_LINE_LAYER_ID,
"background"
)
val bottomLevelRouteLayer = StyleObjectInfo(
BOTTOM_LEVEL_ROUTE_LINE_LAYER_ID,
"background"
)
val mainLayer = StyleObjectInfo(
LAYER_GROUP_1_MAIN,
"line"
)
val style = mockk<Style> {
every { getSource(LAYER_GROUP_1_SOURCE_ID) } returns primaryRouteSource
every { getSource(LAYER_GROUP_2_SOURCE_ID) } returns altRoute1Source
every { getSource(LAYER_GROUP_3_SOURCE_ID) } returns altRoute2Source
every { getSource(WAYPOINT_SOURCE_ID) } returns wayPointSource
every { styleLayers } returns listOf(
bottomLevelRouteLayer,
mainLayer,
topLevelRouteLayer
)
}.also {
mockCheckForLayerInitialization(it)
}

val state: Expected<RouteLineError, RouteLineClearValue> = ExpectedFactory.createValue(
RouteLineClearValue(
primaryRouteFeatureCollection,
listOf(altRoutesFeatureCollection, altRoutesFeatureCollection),
waypointsFeatureCollection
)
)

pauseDispatcher {
val view = MapboxRouteLineView(options)
view.renderClearRouteLineValue(style, state)
view.renderClearRouteLineValue(style, state)
}

verify(exactly = 1) { MapboxRouteLineUtils.initializeLayers(style, options) }
unmockkObject(MapboxRouteLineUtils)
unmockkStatic("com.mapbox.maps.extension.style.sources.SourceUtils")
}

@Test
fun renderTraveledRouteLineUpdate() = coroutineRule.runBlockingTest {
mockkStatic("com.mapbox.maps.extension.style.layers.LayerUtils")
Expand Down Expand Up @@ -1095,6 +1239,7 @@ class MapboxRouteLineViewTest {
view.renderRouteDrawData(style, state)
view.renderRouteDrawData(style2, state2)

verify(exactly = 1) { MapboxRouteLineUtils.initializeLayers(style, options) }
verify(exactly = 1) { style.moveStyleLayer(LAYER_GROUP_1_TRAIL_CASING, any()) }
verify(exactly = 1) { style.moveStyleLayer(LAYER_GROUP_1_TRAIL, any()) }
verify(exactly = 1) { style.moveStyleLayer(LAYER_GROUP_1_CASING, any()) }
Expand Down
Loading