Skip to content
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

Jump to bottom

Adjust the layer position to make puck layer at topmost #3106

Merged
merged 5 commits into from
Jun 29, 2021
Merged

Adjust the layer position to make puck layer at topmost #3106

merged 5 commits into from
Jun 29, 2021

Conversation

ShanMa1991
Copy link
Contributor

Description

This pr is to fix #3098 , because the 3D puck from map side doesn't support the default topmost layer position similar as 2D puck. This pr is to check the 3D puck layer and make it above the maneuvers when we call func navigationMapView.addArrow.

Implementation

Because the 3D puck from map side doesn't support the default topmost layer position similar as 2D puck. This pr is to check the 3D puck layer and make it above the maneuvers when we call func navigationMapView.addArrow

Screenshots or Gifs

@ShanMa1991 ShanMa1991 added bug Something isn’t working UI Work related to visual components, Android Auto, Camera, 3D, voice, etc. labels Jun 22, 2021
@ShanMa1991 ShanMa1991 added this to the v2.0.0 (RC) milestone Jun 22, 2021
@ShanMa1991 ShanMa1991 self-assigned this Jun 22, 2021
@ShanMa1991 ShanMa1991 requested review from MaximAlien and a team June 22, 2021 00:04
MaximAlien
MaximAlien reviewed Jun 22, 2021
View reviewed changes
Sources/MapboxNavigation/NavigationMapView.swift Outdated
@@ -904,7 +905,9 @@ open class NavigationMapView: UIView {
try mapView.mapboxMap.style.addSource(arrowSource, id: NavigationMapView.SourceIdentifier.arrowSource)
arrowLayer.source = NavigationMapView.SourceIdentifier.arrowSource

if mapView.mapboxMap.style.sourceExists(withId: NavigationMapView.LayerIdentifier.waypointCircleLayer) {
if allLayerIds.contains(NavigationMapView.LayerIdentifier.puck3DLayer) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If layer already exists why're we adding it, instead of updating?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is to check whether the 3D puck layer exists, if the 3D puck layer exists, we add the arrowLayer below the puck3D layer. It's the same case for arrowSymbolLayer, to add it below the 3D puck layer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CarPlay issue with maneuvers should be related with the layerID problem as below , add the check of layer existence before set layer position. Similar issue happened for the way point circle layer.

@MaximAlien MaximAlien requested a review from a team June 22, 2021 07:11
@ShanMa1991
Copy link
Contributor Author

The CarPlay has issue with adding maneuvers based on the following error, which leads to the abrupt stop of adding the arrowStrokeLayer, arrowSymbolLayer and arrowSymbolCasingLayer

Failed to perform operation while adding maneuver arrow with error: Cannot find layer with id: 0x000060000070eb00.main.route_line.

ShanMa1991
ShanMa1991 commented Jun 23, 2021
View reviewed changes
Sources/MapboxNavigation/NavigationMapView.swift Outdated
@@ -929,7 +932,11 @@ open class NavigationMapView: UIView {

try mapView.mapboxMap.style.addSource(arrowStrokeSource, id: NavigationMapView.SourceIdentifier.arrowStrokeSource)
arrowStrokeLayer.source = NavigationMapView.SourceIdentifier.arrowStrokeSource
try mapView.mapboxMap.style.addLayer(arrowStrokeLayer, layerPosition: .above(mainRouteLayerIdentifier))
if allLayerIds.contains(mainRouteLayerIdentifier) {
Copy link
Contributor Author

@ShanMa1991 ShanMa1991 Jun 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CarPlay's issue with maneuvers should be related with the layerID problem here. Sometimes the mapView could not find the mainRouteLayerIdentifier. Thus add the check of layer existence before setting layer position. Similar issue happened for the waypoint circle layer.

@S2Ler
Copy link
Contributor

S2Ler commented Jun 24, 2021

Thanks for writing tests 👍

@ShanMa1991 ShanMa1991 mentioned this pull request Jun 28, 2021
Closed
@ShanMa1991 ShanMa1991 linked an issue Jun 29, 2021 that may be closed by this pull request
3D puck layer not at the top most of navigationMapView #3098
Closed
@ShanMa1991 ShanMa1991 requested review from 1ec5 and a team June 29, 2021 19:32
@ShanMa1991 ShanMa1991 changed the title Adjust the layer position to make puck 3D at topmost Adjust the layer position to make puck layer at topmost Jun 29, 2021
@ShanMa1991 ShanMa1991 merged commit d1df20b into release-v2.0 Jun 29, 2021
@ShanMa1991 ShanMa1991 deleted the shan/3D-puck-position-3098 branch June 29, 2021 20:54
This was referenced Jun 29, 2021
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaximAlien MaximAlien Awaiting requested review from MaximAlien

@1ec5 1ec5 Awaiting requested review from 1ec5

Assignees

@ShanMa1991 ShanMa1991

Labels
bug Something isn’t working UI Work related to visual components, Android Auto, Camera, 3D, voice, etc.
Projects
None yet
Milestone
v2.0.0 (RC)
Development

Successfully merging this pull request may close these issues.

3D puck layer not at the top most of navigationMapView
4 participants
@ShanMa1991 @S2Ler @1ec5 @MaximAlien