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

Improve performance of Wayname calculation #2635

Merged
merged 3 commits into from
Sep 21, 2020
Merged

Conversation

avi-c
Copy link
Contributor

@avi-c avi-c commented Sep 16, 2020

Add a predicate filter for the road label MGLLineStyleLayer. Prevent the repeated recalculation of sliced current step line shape.

Fixes #2259.

…repeated recalculation of sliced current step line shape.
@avi-c avi-c requested a review from 1ec5 September 16, 2020 21:24
@1ec5 1ec5 added this to the v1.1.0 milestone Sep 16, 2020
MapboxNavigation/RouteMapViewController.swift Outdated Show resolved Hide resolved
@@ -706,7 +714,6 @@ extension RouteMapViewController: NavigationViewDelegate {
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)!
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, moving the slicing out of the loop should have a measurable impact on areas where there are lots of overlapping lines.

…the predicate filter for walking or cycling profiles.
Copy link
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

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

Suggested changelog entry:

  • Fixed an issue where the current road name label (WayNameLabel) could momentarily show the name of a footpath or bike path even while driving. (#2635)
  • Improved the performance of WayNameLabel in dense, urban areas. (#2635)

@@ -684,6 +684,15 @@ extension RouteMapViewController: NavigationViewDelegate {
streetLabelLayer.lineOpacity = NSExpression(forConstantValue: 1)
streetLabelLayer.lineWidth = NSExpression(forConstantValue: 20)
streetLabelLayer.lineColor = NSExpression(forConstantValue: UIColor.white)

if ![DirectionsProfileIdentifier.walking, DirectionsProfileIdentifier.cycling].contains( router.routeProgress.routeOptions.profileIdentifier) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Btw, I think it’s possible to shorten this line based on the type of profileIdentifier:

Suggested change
if ![DirectionsProfileIdentifier.walking, DirectionsProfileIdentifier.cycling].contains( router.routeProgress.routeOptions.profileIdentifier) {
if ![.walking, .cycling].contains(router.routeProgress.routeOptions.profileIdentifier) {

Comment on lines 690 to 693
let roadPredicates = ["motorway", "motorway_link", "trunk", "trunk_link", "primary", "primary_link", "secondary", "secondary_link", "tertiary", "tertiary_link", "street", "street_limited", "roundabout", "mini_roundabout"].compactMap { roadClass -> NSPredicate? in
return NSPredicate(format: "%K == %@", "class", roadClass)
}
let compoundPredicate = NSCompoundPredicate(orPredicateWithSubpredicates: roadPredicates)
Copy link
Contributor

Choose a reason for hiding this comment

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

Something along these lines should work and would probably perform slightly better:

Suggested change
let roadPredicates = ["motorway", "motorway_link", "trunk", "trunk_link", "primary", "primary_link", "secondary", "secondary_link", "tertiary", "tertiary_link", "street", "street_limited", "roundabout", "mini_roundabout"].compactMap { roadClass -> NSPredicate? in
return NSPredicate(format: "%K == %@", "class", roadClass)
}
let compoundPredicate = NSCompoundPredicate(orPredicateWithSubpredicates: roadPredicates)
let roadPredicates = ["motorway", "motorway_link", "trunk", "trunk_link", "primary", "primary_link", "secondary", "secondary_link", "tertiary", "tertiary_link", "street", "street_limited", "roundabout", "mini_roundabout"]
let compoundPredicate = NSPredicate(format: "class IN %@", roadPredicates)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll adopt these changes. Worth getting the best performance.


if ![DirectionsProfileIdentifier.walking, DirectionsProfileIdentifier.cycling].contains( router.routeProgress.routeOptions.profileIdentifier) {
// filter out to road classes valid for motor transport
let roadPredicates = ["motorway", "motorway_link", "trunk", "trunk_link", "primary", "primary_link", "secondary", "secondary_link", "tertiary", "tertiary_link", "street", "street_limited", "roundabout", "mini_roundabout"].compactMap { roadClass -> NSPredicate? in
Copy link
Contributor

Choose a reason for hiding this comment

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

I’m not sure about mini_roundabout being necessary, since it’s tagged as a node in OpenStreetMap and would thus be a point feature in the Streets source. We’re only querying for polyline features.

@avi-c avi-c merged commit f90e5d1 into master Sep 21, 2020
@avi-c avi-c deleted the ac-FilterRoadnameLabelLayer branch September 21, 2020 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Current road name label can display names of adjacent footpaths
2 participants