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.

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

Expose current way name during active navigation #2548

Closed
avi-c opened this issue Aug 18, 2020 · 8 comments
Closed

Expose current way name during active navigation #2548

avi-c opened this issue Aug 18, 2020 · 8 comments
Assignees
Labels
feature New feature request. op-ex Refactoring, Tech Debt or any other operational excellence work. topic: architecture topic: location UI Work related to visual components, Android Auto, Camera, 3D, voice, etc.
Milestone

Comments

@avi-c
Copy link
Contributor

avi-c commented Aug 18, 2020

The Nav SDK Example app has client side code in RouteMapViewController that utilizes map feature querying to deduce the current road name that the user is driving on. At present any Nav SDK customer needs to duplicate this code or invent another approach in order to calculate the current road name.

This should be something the SDK provides through the delegate pattern used for other nav updates.

Providing this as an abstraction through the SDK also allows us to swap out the implementation if we want to use a new feature of Nav Native or find a more performant way to calculate the road name.

@avi-c
Copy link
Contributor Author

avi-c commented Aug 18, 2020

The newly implemented PassiveLocationManager provides this feature during free drive but we still don't have it during active nav.

@1ec5
Copy link
Contributor

1ec5 commented Aug 18, 2020

The Nav SDK Example app has client side code in RouteMapViewController that utilizes map feature querying to deduce the current road name that the user is driving on.

Here’s the implementation. It’s pretty involved.

func labelCurrentRoadFeature(at location: CLLocation) {
guard let style = mapView.style, let stepShape = router.routeProgress.currentLegProgress.currentStep.shape, !stepShape.coordinates.isEmpty else {
return
}
let closestCoordinate = location.coordinate
let roadLabelStyleLayerIdentifier = "\(identifierNamespace).roadLabels"
var streetsSources: [MGLVectorTileSource] = style.sources.compactMap {
$0 as? MGLVectorTileSource
}.filter {
$0.isMapboxStreets
}
// Add Mapbox Streets if the map does not already have it
if streetsSources.isEmpty {
let source = MGLVectorTileSource(identifier: "com.mapbox.MapboxStreets", configurationURL: URL(string: "mapbox://mapbox.mapbox-streets-v8")!)
style.addSource(source)
streetsSources.append(source)
}
if let mapboxStreetsSource = streetsSources.first, style.layer(withIdentifier: roadLabelStyleLayerIdentifier) == nil {
let streetLabelLayer = MGLLineStyleLayer(identifier: roadLabelStyleLayerIdentifier, source: mapboxStreetsSource)
streetLabelLayer.sourceLayerIdentifier = mapboxStreetsSource.roadLabelLayerIdentifier
streetLabelLayer.lineOpacity = NSExpression(forConstantValue: 1)
streetLabelLayer.lineWidth = NSExpression(forConstantValue: 20)
streetLabelLayer.lineColor = NSExpression(forConstantValue: UIColor.white)
style.insertLayer(streetLabelLayer, at: 0)
}
let userPuck = mapView.convert(closestCoordinate, toPointTo: mapView)
let features = mapView.visibleFeatures(at: userPuck, styleLayerIdentifiers: Set([roadLabelStyleLayerIdentifier]))
var smallestLabelDistance = Double.infinity
var currentName: String?
var currentShieldName: NSAttributedString?
for feature in features {
var allLines: [MGLPolyline] = []
if let line = feature as? MGLPolylineFeature {
allLines.append(line)
} else if let lines = feature as? MGLMultiPolylineFeature {
allLines = lines.polylines
}
for line in allLines {
guard line.pointCount > 0 else { continue }
let featureCoordinates = Array(UnsafeBufferPointer(start: line.coordinates, count: Int(line.pointCount)))
let featurePolyline = LineString(featureCoordinates)
let slicedLine = stepShape.sliced(from: closestCoordinate)!
let lookAheadDistance: CLLocationDistance = 10
guard let pointAheadFeature = featurePolyline.sliced(from: closestCoordinate)!.coordinateFromStart(distance: lookAheadDistance) else { continue }
guard let pointAheadUser = slicedLine.coordinateFromStart(distance: lookAheadDistance) else { continue }
guard let reversedPoint = LineString(featureCoordinates.reversed()).sliced(from: closestCoordinate)!.coordinateFromStart(distance: lookAheadDistance) else { continue }
let distanceBetweenPointsAhead = pointAheadFeature.distance(to: pointAheadUser)
let distanceBetweenReversedPoint = reversedPoint.distance(to: pointAheadUser)
let minDistanceBetweenPoints = min(distanceBetweenPointsAhead, distanceBetweenReversedPoint)
if minDistanceBetweenPoints < smallestLabelDistance {
smallestLabelDistance = minDistanceBetweenPoints
if let line = feature as? MGLPolylineFeature {
let roadNameRecord = roadFeature(for: line)
currentShieldName = roadNameRecord.shieldName
currentName = roadNameRecord.roadName
} else if let line = feature as? MGLMultiPolylineFeature {
let roadNameRecord = roadFeature(for: line)
currentShieldName = roadNameRecord.shieldName
currentName = roadNameRecord.roadName
}
}
}
}
let hasWayName = currentName != nil || currentShieldName != nil
if smallestLabelDistance < 5 && hasWayName {
if let currentShieldName = currentShieldName {
navigationView.wayNameView.attributedText = currentShieldName
} else if let currentName = currentName {
navigationView.wayNameView.text = currentName
}
navigationView.wayNameView.isHidden = false
} else {
navigationView.wayNameView.isHidden = true
}
}
private func roadFeature(for line: MGLFeature) -> (roadName: String?, shieldName: NSAttributedString?) {
var currentShieldName: NSAttributedString?, currentRoadName: String?
if let ref = line.attribute(forKey: "ref") as? String,
let shield = line.attribute(forKey: "shield") as? String,
let reflen = line.attribute(forKey: "reflen") as? Int {
let textColor = roadShieldTextColor(line: line) ?? .black
let imageName = "\(shield)-\(reflen)"
currentShieldName = roadShieldAttributedText(for: ref, textColor: textColor, imageName: imageName)
}
if let roadName = line.attribute(forKey: "name") as? String {
currentRoadName = roadName
}
if let compositeShieldImage = currentShieldName, let roadName = currentRoadName {
let compositeShield = NSMutableAttributedString(string: " \(roadName)")
compositeShield.insert(compositeShieldImage, at: 0)
currentShieldName = compositeShield
}
return (roadName: currentRoadName, shieldName: currentShieldName)
}

Because it’s part of RouteMapViewController, an application that implements a custom UI is unable to take advantage of this implementation.

This implementation is basically a giant workaround for the lack of reliable road name information in the Directions API response. (The response tells us the name of the road at the beginning of each step but doesn’t reflect any name changes between steps.) Even so, we should be able to expose a generic enough interface that we could swap in a more reliable source of road names in the future without breaking backwards compatibility. We’re also tracking some reliability improvements in the feature querying approach in the meantime: #1440.

This should be something the SDK provides through the delegate pattern used for other nav updates.

There is already a NavigationViewControllerDelegate.navigationViewController(_:roadNameAt:) method for the application to provide a road name to label, but not one to tell the application what we think is the current road name.

Ideally, I think this piece of information would fit better in RouteStepProgress. PassiveLocationDataSource exposes it as part of a notification because there isn’t an analogous structure as part of passive navigation. However, feature querying requires access to a currently rendered map, so it would be supremely awkward for something like RouteMapViewController or even NavigationMapView to store state in a route progress model object that RouteController owns.

Maybe in the meantime we can move this feature querying functionality into an extension of NavigationMapView, similar to the approach in #2535, and allow either RouteMapViewController or the application to call the method at the appropriate times.

The newly implemented PassiveLocationManager provides this feature during free drive but we still don't have it during active nav.

This reflects the fact that, unfortunately, MapboxNavigationNative only provides this information during free driving. If the navigator were to provide this information when a route is set, then we could stash the road name in RouteStepProgress and there would be no need for feature querying.

/cc @mapbox/navnative

@1ec5 1ec5 added feature New feature request. op-ex Refactoring, Tech Debt or any other operational excellence work. topic: architecture topic: location labels Aug 18, 2020
@1ec5
Copy link
Contributor

1ec5 commented Mar 22, 2021

The newly implemented PassiveLocationManager provides this feature during free drive but we still don't have it during active nav.

This reflects the fact that, unfortunately, MapboxNavigationNative only provides this information during free driving. If the navigator were to provide this information when a route is set, then we could stash the road name in RouteStepProgress and there would be no need for feature querying.

If this is still the case, we can fall back to the electronic horizon feature added in #2834, which posts a notification on each location update that contains the current road name and ref, among other information. This would be a shoe-in replacement for feature querying, except that the e-horizon notification does not include enough information to choose a route shield. So we probably would need to retain the feature querying step but use the e-horizon information to more reliably choose from among the feature querying results. That would allow us to remove the gangliest part of the feature querying code.

@1ec5
Copy link
Contributor

1ec5 commented Mar 22, 2021

#1440 tracks reimplementing the current road name label atop the information provided by PassiveLocationManager and electronic horizon notifications. That work is somewhat orthogonal to making the feature querying logic more reusable, but the two tasks would probably happen in tandem.

@truburt truburt added wip and removed wip labels Apr 2, 2021
@1ec5 1ec5 modified the milestones: v2.0.0 (GA), v2.0.0 (RC) May 4, 2021
@1ec5 1ec5 modified the milestones: v2.0.0 (RC), v2.1 (beta) Jun 1, 2021
@1ec5
Copy link
Contributor

1ec5 commented Oct 14, 2021

#2902 broke out the logic for updating WayNameView so that it can be reused outside of NavigationViewController. Now that that’s out of the way, the simplest approach would be to make NavigationMapView.labelCurrentRoad(at:suggestedName:for:) public:

func labelCurrentRoad(at rawLocation: CLLocation, suggestedName roadName: String?, for snappedLocation: CLLocation? = nil) {

While we’re at it, we could streamline the method to take a single location argument, pushing this coalescing expression up to the method’s callers:

labelCurrentRoadFeature(at: snappedLocation ?? rawLocation)

@MaximAlien MaximAlien self-assigned this Nov 15, 2021
@MaximAlien MaximAlien added the UI Work related to visual components, Android Auto, Camera, 3D, voice, etc. label Nov 15, 2021
@MaximAlien MaximAlien modified the milestones: v2.1-rc, v2.2 Nov 29, 2021
@MaximAlien MaximAlien modified the milestones: v2.2, v2.3 Jan 20, 2022
@1ec5
Copy link
Contributor

1ec5 commented Feb 4, 2022

@MaximAlien @ShanMa1991, do we need to expose anything additional so that custom UIs can populate a current road name view as easily as NavigationViewController does?

@ShanMa1991
Copy link
Contributor

I think we need to expose the shield related information. Because if we switch to use the Mapbox designed shield, we would deprecate the use of map feature query. If we don't provide the shield information, there won't be useful shield for the road labeling.
And also right now we have already exposed the road name through Notification in RouteController and PassiveLocationManger, we'd better also add the road shield to it as well to support the road labeling under passive and active navigation.

@1ec5
Copy link
Contributor

1ec5 commented Feb 18, 2022

The Mapbox-designed shield work in #1119 will completely remove the functionality described in #2548 (comment), so there’s no need to expose it publicly to developers in the meantime.

@1ec5 1ec5 closed this as completed Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature request. op-ex Refactoring, Tech Debt or any other operational excellence work. topic: architecture topic: location UI Work related to visual components, Android Auto, Camera, 3D, voice, etc.
Projects
None yet
Development

No branches or pull requests

5 participants