Fix the route line layer position issue after rerouting. #4062

ShanMa1991 · 2022-08-09T21:26:10Z

Description

This Pr is to fix the issue of route line layer position issue after rerouting.

Implementation

When finding the topmost non-symbol layer for route line, explicit check whether the layer is the label layer . During style loading, some label layers exist above symbol layers, such as poi label copy as a circle layer but above the symbol layer of poi label. So it will cause the NavigationMapView.layerPosition(for:route:customLayerPosition:) found the wrong topmost non-symbol layer, especially for route line.
When adding casing layer and alternative route layer, check whether the parent layer added to the style already.
When adding route line layers, remove all the continuous alternative route layers first.
Added test for the custom layer position of route line and the non-symbol label layer .

Use cases

Call NavigationMapView.show(_:layerPosition:legIndex:) to provide custom layer position for route line in both CarPlay and mobile.
During active navigation, when alternative routes update, call NavigationMapView.show(continuousAlternatives:). When reroute, refresh, routeProgress update, call NavigationMapView.updateRouteLine(routeProgress:coordinate:shouldRedraw:). They will keep the custom layer position for route line in both mobile and CarPlay.

Screenshots or Gifs

ShanMa1991 · 2022-08-09T22:11:16Z

Sources/MapboxNavigation/NavigationMapView+VanishingRouteLine.swift

@@ -445,7 +445,7 @@ extension NavigationMapView {
    func routeLineRestrictionsGradient(_ restrictionFeatures: [Turf.Feature]) -> [Double: UIColor] {
        // If there's no restricted feature, hide the restricted route line layer.
        guard restrictionFeatures.count > 0 else {
-            let gradientStops: [Double: UIColor] = [0.0: traversedRouteColor]
+            let gradientStops: [Double: UIColor] = [0.0: .defaultTraversedRouteColor]


Avoid the color customization, to hide the whole restricted area layer if it's empty.

ShanMa1991 · 2022-08-09T22:12:23Z

Sources/MapboxNavigation/NavigationMapView.swift

@@ -1792,6 +1793,7 @@ open class NavigationMapView: UIView {
                // find the topmost non symbol layer for layerIdentifier in lowermostSymbolLayers.
                if targetLayer == nil,
                   layerInfo.type.rawValue != "symbol",
+                   !layerInfo.id.contains("copy"),


Some style has copy layer along with other layers at the same strata. It will lead to the wrong topmost non symbol layer.

Style layer IDs can be arbitrarily named by the style designer. The presence of “copy” means nothing in particular, just that someone clicked Duplicate Layer in Mapbox Studio at some point, so we shouldn’t hard-code this magic value.

Is there anything else we can look for to determine whether a non-symbol layer should be treated like a symbol layer for this purpose? The closest thing I can think of would be to special-case a circle layer whose circlePitchAlignment is viewport, which tends to make the circle look like a symbol. A common use of circle layers is to render things like culs-de-sac at realistic sizes, but those would have a circlePitchAlignment of map.

Otherwise, if these extra heuristics don’t work reliably enough, the application should specify a custom layer position.

Thanks for the suggestion, the copy is replaced with a general label in case of this situation would happen. And also the custom layer position for the route line is supported during active navigation and when route line related properties change.

The navigation SDK shouldn’t impose any requirements on the layer ID. The layer ID is intended to be freeform text. If a team’s iOS developer is merely integrating a style that a designer authored in Mapbox Studio, we have no opportunity to communicate to the style designer that this exceptional layer should have label in the ID. On the other hand, if the iOS developer is adding the layer using runtime styling, they might still want to call it something that doesn’t mention “label”.

Do we know more about the use cases where a circle layer would imitate a symbol layer? Can we rely on circlePitchAlignment to be viewport?

I removed the id check. But I don't think the circle layer is the only copy layer. The check around the viewport is not definition. When testing the test app, if the styles are not fully loaded, there may be random copy layers.

In Nav SDK, on both mobile and CarPlay, we have listeners for MapView.mapboxMap.onNext(event: .styleLoaded, handler:), when the style changes, The route line layers are updated and regenerated.
If the style is not fully loaded, there may be some random copy layers, or the right layer position has been found but we couldn't add it.

So the best way to solve this issue, is that the users provide their own customLayerPosition for the route line. This PR would remember the custom layer position for route line in both mobile and CarPlay. When the style changes, the route line would use the previous remembered custom layer position for the new style by default.

I opened #4079 to track enhancing the default positioning heuristic to account for circle layers.

I adopted #4079 to check the circle layer as a label layer. Route line will be placed below them as default.

CHANGELOG.md

1ec5 · 2022-08-11T02:11:14Z

Sources/MapboxNavigation/NavigationMapView.swift

@@ -1792,6 +1793,7 @@ open class NavigationMapView: UIView {
                // find the topmost non symbol layer for layerIdentifier in lowermostSymbolLayers.
                if targetLayer == nil,
                   layerInfo.type.rawValue != "symbol",
+                   !layerInfo.id.contains("copy"),


Style layer IDs can be arbitrarily named by the style designer. The presence of “copy” means nothing in particular, just that someone clicked Duplicate Layer in Mapbox Studio at some point, so we shouldn’t hard-code this magic value.

Is there anything else we can look for to determine whether a non-symbol layer should be treated like a symbol layer for this purpose? The closest thing I can think of would be to special-case a circle layer whose circlePitchAlignment is viewport, which tends to make the circle look like a symbol. A common use of circle layers is to render things like culs-de-sac at realistic sizes, but those would have a circlePitchAlignment of map.

Otherwise, if these extra heuristics don’t work reliably enough, the application should specify a custom layer position.

ShanMa1991 · 2022-08-15T21:51:44Z

Sources/MapboxNavigation/NavigationMapView.swift

+    /**
+     A custom route line layer position.
+     */
+    var customRouteLineLayerPosition: MapboxMaps.LayerPosition? = nil


customRouteLineLayerPosition is used to remember the custom layer position for route line during active navigation or when route line related properties change.

mapbox-github-ci-issues-public-1 · 2022-08-16T21:07:51Z

Breaking Changes in MapboxCoreNavigation

Breaking API Changes

`MapboxNavigationService`

removed method: init(routeResponse:routeIndex:routeOptions:customRoutingProvider:credentials:locationSource:eventsManagerType:simulating:routerType:) in MapboxNavigationService

mapbox-github-ci-issues-public-1 · 2022-08-16T22:32:39Z

Breaking Changes in MapboxCoreNavigation

Breaking API Changes

`MapboxNavigationService`

removed method: init(routeResponse:routeIndex:routeOptions:customRoutingProvider:credentials:locationSource:eventsManagerType:simulating:routerType:) in MapboxNavigationService

mapbox-github-ci-issues-public-1 · 2022-08-16T23:57:25Z

Breaking Changes in MapboxCoreNavigation

Breaking API Changes

`MapboxNavigationService`

removed method: init(routeResponse:routeIndex:routeOptions:customRoutingProvider:credentials:locationSource:eventsManagerType:simulating:routerType:) in MapboxNavigationService

CHANGELOG.md

…t nonsymbol layer.

… area.

…oute line.

…ine position.

ShanMa1991 · 2022-08-17T20:02:26Z

Sources/MapboxNavigation/NavigationMapView.swift

@@ -1794,6 +1803,11 @@ open class NavigationMapView: UIView {
                   layerInfo.type.rawValue != "symbol",
                   let sourceLayer = mapView.mapboxMap.style.layerProperty(for: layerInfo.id, property: "source-layer").value as? String,
                   !sourceLayer.isEmpty {
+                    if layerInfo.type.rawValue == "circle",


According to #4079 , add check for circle type label layer, route line will be placed below them.

1ec5 · 2022-08-17T21:00:21Z

Sources/MapboxNavigation/NavigationMapView.swift

@@ -1794,6 +1803,11 @@ open class NavigationMapView: UIView {
                   layerInfo.type.rawValue != "symbol",
                   let sourceLayer = mapView.mapboxMap.style.layerProperty(for: layerInfo.id, property: "source-layer").value as? String,
                   !sourceLayer.isEmpty {
+                    if layerInfo.type.rawValue == "circle",
+                       let isPersistentCircle = try? mapView.mapboxMap.style.isPersistentLayer(id: layerInfo.id),
+                       !isPersistentCircle {


Is this logic correct? I would’ve assumed that any kind of annotation or data visualization atop the map would use a persistent circle layer, so a persistent circle layer would need to be skipped, but a non-persistent circle layer would be treated as the target layer.

1ec5 · 2022-08-17T21:05:18Z

Sources/MapboxNavigation/NavigationMapView.swift

@@ -1794,6 +1803,11 @@ open class NavigationMapView: UIView {
                   layerInfo.type.rawValue != "symbol",


Nesting the if statement below gets confusing. I think this would more clearly communicate the intention:

(layerInfo.type.rawValue != "symbol" and !(layerInfo.type.rawValue == "circle" and mapView.mapboxMap.style.isPersistentLayer(id: layerInfo.id)))

ShanMa1991 added the UI Work related to visual components, Android Auto, Camera, 3D, voice, etc. label Aug 9, 2022

ShanMa1991 added this to the v2.7 milestone Aug 9, 2022

ShanMa1991 self-assigned this Aug 9, 2022

ShanMa1991 commented Aug 9, 2022

View reviewed changes

ShanMa1991 requested a review from a team August 9, 2022 22:31

1ec5 reviewed Aug 11, 2022

View reviewed changes

ShanMa1991 force-pushed the shan/finding-non-symbole-layer branch from a1bf830 to 4c65531 Compare August 15, 2022 21:49

ShanMa1991 commented Aug 15, 2022

View reviewed changes

ShanMa1991 requested review from 1ec5 and a team August 15, 2022 22:10

ShanMa1991 force-pushed the shan/finding-non-symbole-layer branch from 0428607 to ca737c5 Compare August 16, 2022 20:55

ShanMa1991 modified the milestones: v2.7, v2.8 Aug 17, 2022

1ec5 mentioned this pull request Aug 17, 2022

Automatic route line layer positioning should skip circle layers being used as symbol layers #4079

Closed

1ec5 approved these changes Aug 17, 2022

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

ShanMa1991 added 10 commits August 17, 2022 11:33

Fix the route line layer position issue when finding the wrong topmos…

96d3978

…t nonsymbol layer.

Update the changeLog.

a8820a7

Avoid the customization of traversedRouteColor to show the restricted…

d02467f

… area.

Support custom layer position for route line in active navigation.

759b8cd

Update the changeLog.

1567fea

Add example for mobile and CarPlay to use custom layer position for r…

3cf5fed

…oute line.

Remove the requirement check for label id.

8aa05dd

Stop breaking the layer finding loop.

afeab70

Add test when copy layer existed.

2f87791

Add check for circle label when finding non-symbol layers for route l…

6f9d2dc

…ine position.

ShanMa1991 force-pushed the shan/finding-non-symbole-layer branch from 2d89b9d to 6f9d2dc Compare August 17, 2022 19:01

target to 2.8 release.

4dcfc58

ShanMa1991 commented Aug 17, 2022

View reviewed changes

ShanMa1991 merged commit 58826ff into main Aug 17, 2022

ShanMa1991 deleted the shan/finding-non-symbole-layer branch August 17, 2022 20:18

1ec5 reviewed Aug 17, 2022

View reviewed changes

ShanMa1991 mentioned this pull request Sep 13, 2022

Update the check for circleLayer for default route line position #4112

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the route line layer position issue after rerouting. #4062

Fix the route line layer position issue after rerouting. #4062

ShanMa1991 commented Aug 9, 2022 •

edited

ShanMa1991 Aug 9, 2022

ShanMa1991 Aug 9, 2022

1ec5 Aug 11, 2022 •

edited

ShanMa1991 Aug 15, 2022

1ec5 Aug 15, 2022

ShanMa1991 Aug 16, 2022

1ec5 Aug 17, 2022

ShanMa1991 Aug 17, 2022

1ec5 Aug 11, 2022 •

edited

ShanMa1991 Aug 15, 2022

mapbox-github-ci-issues-public-1 bot commented Aug 16, 2022

mapbox-github-ci-issues-public-1 bot commented Aug 16, 2022

mapbox-github-ci-issues-public-1 bot commented Aug 16, 2022

ShanMa1991 Aug 17, 2022

1ec5 Aug 17, 2022

1ec5 Aug 17, 2022 •

edited

		@@ -1794,6 +1803,11 @@ open class NavigationMapView: UIView {
		layerInfo.type.rawValue != "symbol",

Fix the route line layer position issue after rerouting. #4062

Fix the route line layer position issue after rerouting. #4062

Conversation

ShanMa1991 commented Aug 9, 2022 • edited

Description

Implementation

Use cases

Screenshots or Gifs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 Aug 11, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 Aug 11, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mapbox-github-ci-issues-public-1 bot commented Aug 16, 2022

Breaking Changes in MapboxCoreNavigation

Breaking API Changes

MapboxNavigationService

mapbox-github-ci-issues-public-1 bot commented Aug 16, 2022

Breaking Changes in MapboxCoreNavigation

Breaking API Changes

MapboxNavigationService

mapbox-github-ci-issues-public-1 bot commented Aug 16, 2022

Breaking Changes in MapboxCoreNavigation

Breaking API Changes

MapboxNavigationService

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 Aug 17, 2022 • edited

Choose a reason for hiding this comment

ShanMa1991 commented Aug 9, 2022 •

edited

1ec5 Aug 11, 2022 •

edited

1ec5 Aug 11, 2022 •

edited

`MapboxNavigationService`

`MapboxNavigationService`

`MapboxNavigationService`

1ec5 Aug 17, 2022 •

edited