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

update turf #91

Merged
merged 7 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
* Custom location snapping in the `RouteController` via the delegate
* Fix: If the directions API endpoint doesn't include audio instructions, `didArrive:` would never be called.
* Merged in <https://github.com/maplibre/maplibre-navigation-ios/pull/72>
* Updated "turf" geometry library from 0.2.2 to 2.8.0
* Merged in https://github.com/maplibre/maplibre-navigation-ios/pull/91

## 3.0.0 (Jun 15, 2024)
* The `speak` method in `RouteVoiceController` can be used without a given `RouteProgress` or the `RouteProgress` can explicitly ignored so that it will not be added to the voice instruction.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@
{
"identity" : "turf-swift",
"kind" : "remoteSourceControl",
"location" : "https://github.com/flitsmeister/turf-swift",
"location" : "https://github.com/mapbox/turf-swift.git",
"state" : {
"revision" : "b05b4658d1b48eac4127a0d9ebbb5a6f965a8251",
"version" : "0.2.2"
"revision" : "213050191cfcb3d5aa76e1fa90c6ff1e182a42ca",
"version" : "2.8.0"
}
}
],
Expand Down
25 changes: 14 additions & 11 deletions MapboxCoreNavigation/CLLocation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,27 +61,29 @@ extension CLLocation {
Returns a Boolean value indicating whether the receiver is within a given distance of a route step.
*/
func isWithin(_ maximumDistance: CLLocationDistance, of routeStep: RouteStep) -> Bool {
guard let closestCoordinate = Polyline(routeStep.coordinates!).closestCoordinate(to: coordinate) else {
guard let closestCoordinate = LineString(routeStep.coordinates!).closestCoordinate(to: coordinate) else {
return false
}
return closestCoordinate.distance < maximumDistance
let distance = closestCoordinate.coordinate.distance(to: self.coordinate)
return distance < maximumDistance
}

// MARK: - Route Snapping

func snapped(to legProgress: RouteLegProgress) -> CLLocation? {
let coords = self.coordinates(for: legProgress)

guard let closest = Polyline(coords).closestCoordinate(to: coordinate) else { return nil }
guard let closest = LineString(coords).closestCoordinate(to: coordinate) else { return nil }
guard let calculatedCourseForLocationOnStep = interpolatedCourse(along: coords) else { return nil }

let userCourse = calculatedCourseForLocationOnStep
let userCoordinate = closest.coordinate
guard let firstCoordinate = legProgress.leg.steps.first?.coordinates?.first else { return nil }

guard self.shouldSnapCourse(toRouteWith: calculatedCourseForLocationOnStep, distanceToFirstCoordinateOnLeg: coordinate.distance(to: firstCoordinate)) else { return nil }

guard closest.distance <= (RouteControllerUserLocationSnappingDistance + horizontalAccuracy) else {

let distanceFromLeg = closest.coordinate.distance(to: self.coordinate)
guard distanceFromLeg <= (RouteControllerUserLocationSnappingDistance + horizontalAccuracy) else {
return nil
}

Expand Down Expand Up @@ -121,12 +123,12 @@ extension CLLocation {
Given a location and a series of coordinates, compute what the course should be for a the location.
*/
func interpolatedCourse(along coordinates: [CLLocationCoordinate2D]) -> CLLocationDirection? {
let nearByPolyline = Polyline(coordinates)
let nearByPolyline = LineString(coordinates)

guard let closest = nearByPolyline.closestCoordinate(to: coordinate) else { return nil }

let slicedLineBehind = Polyline(coordinates.reversed()).sliced(from: closest.coordinate, to: coordinates.reversed().last)
let slicedLineInFront = nearByPolyline.sliced(from: closest.coordinate, to: coordinates.last)
guard let slicedLineBehind = LineString(coordinates.reversed()).sliced(from: closest.coordinate, to: coordinates.reversed().last) else { return nil }
guard let slicedLineInFront = nearByPolyline.sliced(from: closest.coordinate, to: coordinates.last) else { return nil }
Comment on lines +130 to +131
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was returning nil here not needed before?

Copy link
Collaborator Author

@michaelkirk michaelkirk Aug 15, 2024

Choose a reason for hiding this comment

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

A good question!

The proximate answer is that the sliced method signature changed. Previously, if you had a LineString with 0 coordinates, if you tried to slice it, it would return another LineString with 0 coordinates. Now it returns nil.

Before:
mapbox/turf-swift@b05b465...v2.8.0#diff-809e6d4a01e9482c2a2ac457a5394236ec89bd5355c5ab09fddb69f686225aa2L175

public func sliced(from start: CLLocationCoordinate2D? = nil, to end: CLLocationCoordinate2D? = nil) -> LineString {
    // Ported from https://github.com/Turfjs/turf/blob/142e137ce0c758e2825a260ab32b24db0aa19439/packages/turf-line-slice/index.js
    guard !coordinates.isEmpty else {
        return LineString([])
    }
    ....

After:
mapbox/turf-swift@b05b465...v2.8.0#diff-9de11b3a56fd1de998fe9ec9a4b133b503af0298afe0008984076e5adb200ca9R276

public func sliced(from start: LocationCoordinate2D? = nil, to end: LocationCoordinate2D? = nil) -> LineString? {
    // Ported from https://github.com/Turfjs/turf/blob/142e137ce0c758e2825a260ab32b24db0aa19439/packages/turf-line-slice/index.js
    guard !coordinates.isEmpty else { return nil }

Seems a little arbitrary, but whatever. 🤷

In any case, could this conceivably affect behavior?

Well, a few lines below your comment is another line:

guard let pointBehind = slicedLineBehind.coordinateFromStart(distance: userDistanceBuffer) else { return nil }

This method in turn ultimately calls:

public func indexedCoordinateFromStart(distance: CLLocationDistance) -> IndexedCoordinate? {
    guard let firstCoordinate = coordinates.first else {
        return nil
    }
    ...
}

This is true for both the old and the new version of turf.

So to summarize, the difference only applies in the degenerate case of getting an empty LineString as input. And in that case, the only difference is we'd now return nil a few lines earlier than we would have returned nil before, but we'll still ultimately return nil.

Bottom line: I don't think there could be an observable change in behavior from this.

let userDistanceBuffer: CLLocationDistance = max(speed * RouteControllerDeadReckoningTimeInterval / 2, RouteControllerUserLocationSnappingDistance / 2)

guard let pointBehind = slicedLineBehind.coordinateFromStart(distance: userDistanceBuffer) else { return nil }
Expand All @@ -145,10 +147,11 @@ extension CLLocation {

let averageRelativeAngle: Double
// User is at the beginning of the route, there is no closest point behind the user.
= if pointBehindClosest.distance <= 0, pointAheadClosest.distance > 0 {
= if pointBehindClosest.coordinate.distance(to: pointBehind) <= 0, pointAheadClosest.coordinate.distance(to: pointAhead) > 0 {
relativeAnglepointAhead
// User is at the end of the route, there is no closest point in front of the user.
} else if pointAheadClosest.distance <= 0, pointBehindClosest.distance > 0 {
} else if pointAheadClosest.coordinate.distance(to: pointAhead) <= 0,
pointBehindClosest.coordinate.distance(to: pointBehind) > 0 {
relativeAnglepointBehind
} else {
(relativeAnglepointBehind + relativeAnglepointAhead) / 2
Expand Down
16 changes: 8 additions & 8 deletions MapboxCoreNavigation/RouteController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@
self.userSnapToStepDistanceFromManeuver = nil
return
}
self.userSnapToStepDistanceFromManeuver = Polyline(coordinates).distance(from: coordinate)
self.userSnapToStepDistanceFromManeuver = LineString(coordinates).distance(from: coordinate)
}

/**
Expand Down Expand Up @@ -303,7 +303,7 @@
@objc func interpolateLocation() {
guard let location = locationManager.lastKnownLocation else { return }
guard let coordinates = routeProgress.route.coordinates else { return }
let polyline = Polyline(coordinates)
let polyline = LineString(coordinates)

let distance = location.speed as CLLocationDistance

Expand Down Expand Up @@ -380,9 +380,9 @@

self.updateIntersectionIndex(for: currentStepProgress)
// Notify observers if the step’s remaining distance has changed.
let polyline = Polyline(routeProgress.currentLegProgress.currentStep.coordinates!)
if let closestCoordinate = polyline.closestCoordinate(to: location.coordinate) {
let remainingDistance = polyline.distance(from: closestCoordinate.coordinate)
let polyline = LineString(routeProgress.currentLegProgress.currentStep.coordinates!)
if let closestCoordinate = polyline.closestCoordinate(to: location.coordinate),
let remainingDistance = polyline.distance(from: closestCoordinate.coordinate) {
let distanceTraveled = currentStep.distance - remainingDistance
currentStepProgress.distanceTraveled = distanceTraveled
NotificationCenter.default.post(name: .routeControllerProgressDidChange, object: self, userInfo: [
Expand Down Expand Up @@ -671,7 +671,7 @@
if let _ = error { return }
guard let httpResponse = response as? HTTPURLResponse, httpResponse.statusCode == 200 else { return }

guard let data, let currentVersion = String(data: data, encoding: .utf8)?.trimmingCharacters(in: .newlines) else { return }

Check warning on line 674 in MapboxCoreNavigation/RouteController.swift

View workflow job for this annotation

GitHub Actions / Code Style

Prefer using UTF-8 encoded strings when converting between `String` and `Data` (non_optional_string_data_conversion)

if latestVersion != currentVersion {
let updateString = NSLocalizedString("UPDATE_AVAILABLE", bundle: .mapboxCoreNavigation, value: "Mapbox Navigation SDK for iOS version %@ is now available.", comment: "Inform developer an update is available")
Expand Down Expand Up @@ -727,7 +727,7 @@
self.routeProgress.currentLegProgress.currentStepProgress.intersectionsIncludingUpcomingManeuverIntersection = intersections

if let upcomingIntersection = routeProgress.currentLegProgress.currentStepProgress.upcomingIntersection {
self.routeProgress.currentLegProgress.currentStepProgress.userDistanceToUpcomingIntersection = Polyline(currentStepProgress.step.coordinates!).distance(from: location.coordinate, to: upcomingIntersection.location)
self.routeProgress.currentLegProgress.currentStepProgress.userDistanceToUpcomingIntersection = LineString(currentStepProgress.step.coordinates!).distance(from: location.coordinate, to: upcomingIntersection.location)
}

if self.routeProgress.currentLegProgress.currentStepProgress.intersectionDistances == nil {
Expand Down Expand Up @@ -828,8 +828,8 @@

func updateIntersectionDistances() {
if let coordinates = routeProgress.currentLegProgress.currentStep.coordinates, let intersections = routeProgress.currentLegProgress.currentStep.intersections {
let polyline = Polyline(coordinates)
let distances: [CLLocationDistance] = intersections.map { polyline.distance(from: coordinates.first, to: $0.location) }
let polyline = LineString(coordinates)
let distances: [CLLocationDistance] = intersections.compactMap { polyline.distance(from: coordinates.first, to: $0.location) }
self.routeProgress.currentLegProgress.currentStepProgress.intersectionDistances = distances
}
}
Expand Down
9 changes: 5 additions & 4 deletions MapboxCoreNavigation/RouteProgress.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
@objc public var legIndex: Int {
didSet {
assert(self.legIndex >= 0 && self.legIndex < self.route.legs.endIndex)
// TODO: Set stepIndex to 0 or last index based on whether leg index was incremented or decremented.

Check warning on line 26 in MapboxCoreNavigation/RouteProgress.swift

View workflow job for this annotation

GitHub Actions / Code Style

TODOs should be resolved (Set stepIndex to 0 or last ind...) (todo)
self.currentLegProgress = RouteLegProgress(leg: self.currentLeg)
}
}
Expand Down Expand Up @@ -368,17 +368,18 @@

for (currentStepIndex, step) in remainingSteps.enumerated() {
guard let coords = step.coordinates else { continue }
guard let closestCoordOnStep = Polyline(coords).closestCoordinate(to: coordinate) else { continue }
guard let closestCoordOnStep = LineString(coords).closestCoordinate(to: coordinate) else { continue }
let foundIndex = currentStepIndex + self.stepIndex

let distanceFromLine = closestCoordOnStep.coordinate.distance(to: coordinate)
// First time around, currentClosest will be `nil`.
michaelkirk marked this conversation as resolved.
Show resolved Hide resolved
guard let currentClosestDistance = currentClosest?.distance else {
currentClosest = (index: foundIndex, distance: closestCoordOnStep.distance)
currentClosest = (index: foundIndex, distance: distanceFromLine)
continue
}

if closestCoordOnStep.distance < currentClosestDistance {
currentClosest = (index: foundIndex, distance: closestCoordOnStep.distance)
if distanceFromLine < currentClosestDistance {
currentClosest = (index: foundIndex, distance: distanceFromLine)
}
}

Expand Down
8 changes: 4 additions & 4 deletions MapboxCoreNavigation/SimulatedLocationManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ open class SimulatedLocationManager: NavigationLocationManager {
@objc fileprivate func tick() {
NSObject.cancelPreviousPerformRequests(withTarget: self, selector: #selector(self.tick), object: nil)

let polyline = Polyline(routeLine)
let polyline = LineString(routeLine)

guard let newCoordinate = polyline.coordinateFromStart(distance: currentDistance) else {
return
Expand All @@ -177,12 +177,12 @@ open class SimulatedLocationManager: NavigationLocationManager {
let distanceToClosest = closestLocation.distance(from: CLLocation(newCoordinate))

let distance = min(max(distanceToClosest, 10), safeDistance)
let coordinatesNearby = polyline.trimmed(from: newCoordinate, distance: 100).coordinates
guard let coordinatesNearby = polyline.trimmed(from: newCoordinate, distance: 100)?.coordinates else { return }

// Simulate speed based on expected segment travel time
if let expectedSegmentTravelTimes = routeProgress?.currentLeg.expectedSegmentTravelTimes,
let coordinates = routeProgress?.route.coordinates,
let closestCoordinateOnRoute = Polyline(routeProgress!.route.coordinates!).closestCoordinate(to: newCoordinate),
let closestCoordinateOnRoute = LineString(routeProgress!.route.coordinates!).closestCoordinate(to: newCoordinate),
let nextCoordinateOnRoute = coordinates.after(element: coordinates[closestCoordinateOnRoute.index]),
let time = expectedSegmentTravelTimes.optional[closestCoordinateOnRoute.index] {
let distance = coordinates[closestCoordinateOnRoute.index].distance(to: nextCoordinateOnRoute)
Expand Down
4 changes: 2 additions & 2 deletions MapboxCoreNavigationTests/RouteControllerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class RouteControllerTests: XCTestCase {
navigation.locationManager(navigation.locationManager, didUpdateLocations: [firstLocation])
XCTAssertEqual(navigation.location!.coordinate, firstLocation.coordinate, "Check snapped location is working")

let futureCoord = Polyline(navigation.routeProgress.currentLegProgress.nearbyCoordinates).coordinateFromStart(distance: 100)!
let futureCoord = LineString(navigation.routeProgress.currentLegProgress.nearbyCoordinates).coordinateFromStart(distance: 100)!
let futureInaccurateLocation = CLLocation(coordinate: futureCoord, altitude: 0, horizontalAccuracy: 1, verticalAccuracy: 200, course: 0, speed: 5, timestamp: Date())

navigation.locationManager(navigation.locationManager, didUpdateLocations: [futureInaccurateLocation])
Expand All @@ -165,7 +165,7 @@ class RouteControllerTests: XCTestCase {
let navigation = RouteController(along: route, directions: directions)
let firstCoord = navigation.routeProgress.currentLegProgress.nearbyCoordinates.first!
let firstLocation = CLLocation(latitude: firstCoord.latitude, longitude: firstCoord.longitude)
let coordNearStart = Polyline(navigation.routeProgress.currentLegProgress.nearbyCoordinates).coordinateFromStart(distance: 10)!
let coordNearStart = LineString(navigation.routeProgress.currentLegProgress.nearbyCoordinates).coordinateFromStart(distance: 10)!

navigation.locationManager(navigation.locationManager, didUpdateLocations: [firstLocation])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ private extension TunnelIntersectionManagerTests {
for routeController: RouteController,
intersection: Intersection,
distance: CLLocationDistance? = 200) -> CLLocation {
let polyline = Polyline(routeController.routeProgress.currentLegProgress.currentStep.coordinates!)
let polyline = LineString(routeController.routeProgress.currentLegProgress.currentStep.coordinates!)
let newLocation = CLLocationCoordinate2D(latitude: coordinate.latitude,
longitude: coordinate.longitude).coordinate(
at: distance!,
Expand Down
31 changes: 19 additions & 12 deletions MapboxNavigation/NavigationMapView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -600,16 +600,19 @@
let minimumZoomLevel: Float = 14.5

let shaftLength = max(min(30 * metersPerPoint(atLatitude: maneuverCoordinate.latitude), 30), 10)
let polyline = Polyline(routeCoordinates)
let shaftCoordinates = Array(polyline.trimmed(from: maneuverCoordinate, distance: -shaftLength).coordinates.reversed()
+ polyline.trimmed(from: maneuverCoordinate, distance: shaftLength).coordinates.suffix(from: 1))
let polyline = LineString(routeCoordinates)
guard let beforeTrimmedPolyline = polyline.trimmed(from: maneuverCoordinate, distance: -shaftLength),
let afterTrimmedPolyline = polyline.trimmed(from: maneuverCoordinate, distance: shaftLength) else {
return
}
let shaftCoordinates = Array(beforeTrimmedPolyline.coordinates.reversed() + afterTrimmedPolyline.coordinates.suffix(from: 1))
if shaftCoordinates.count > 1 {
var shaftStrokeCoordinates = shaftCoordinates
let shaftStrokePolyline = ArrowStrokePolyline(coordinates: &shaftStrokeCoordinates, count: UInt(shaftStrokeCoordinates.count))
let shaftDirection = shaftStrokeCoordinates[shaftStrokeCoordinates.count - 2].direction(to: shaftStrokeCoordinates.last!)
let maneuverArrowStrokePolylines = [shaftStrokePolyline]
let shaftPolyline = ArrowFillPolyline(coordinates: shaftCoordinates, count: UInt(shaftCoordinates.count))

let arrowShape = MLNShapeCollection(shapes: [shaftPolyline])
let arrowStrokeShape = MLNShapeCollection(shapes: maneuverArrowStrokePolylines)

Expand Down Expand Up @@ -748,7 +751,7 @@
}
}

// TODO: Change to point-based distance calculation

Check warning on line 754 in MapboxNavigation/NavigationMapView.swift

View workflow job for this annotation

GitHub Actions / Code Style

TODOs should be resolved (Change to point-based distance...) (todo)
private func waypoints(on routes: [Route], closeTo point: CGPoint) -> [Waypoint]? {
let tapCoordinate = convert(point, toCoordinateFrom: self)
let multipointRoutes = routes.filter { $0.routeOptions.waypoints.count >= 3 }
Expand Down Expand Up @@ -781,17 +784,17 @@
let closest = routes.sorted { left, right -> Bool in

// existance has been assured through use of filter.
let leftLine = Polyline(left.coordinates!)
let rightLine = Polyline(right.coordinates!)
let leftDistance = leftLine.closestCoordinate(to: tapCoordinate)!.distance
let rightDistance = rightLine.closestCoordinate(to: tapCoordinate)!.distance
let leftLine = LineString(left.coordinates!)
let rightLine = LineString(right.coordinates!)
let leftDistance = leftLine.closestCoordinate(to: tapCoordinate)!.coordinate.distance(to: tapCoordinate)
let rightDistance = rightLine.closestCoordinate(to: tapCoordinate)!.coordinate.distance(to: tapCoordinate)

return leftDistance < rightDistance
}

// filter closest coordinates by which ones are under threshold.
let candidates = closest.filter {
let closestCoordinate = Polyline($0.coordinates!).closestCoordinate(to: tapCoordinate)!.coordinate
let closestCoordinate = LineString($0.coordinates!).closestCoordinate(to: tapCoordinate)!.coordinate
let closestPoint = self.convert(closestCoordinate, toPointTo: self)

return closestPoint.distance(to: point) < self.tapGestureDistanceThreshold
Expand Down Expand Up @@ -1037,7 +1040,7 @@
for (stepIndex, step) in leg.steps.enumerated() {
for instruction in step.instructionsSpokenAlongStep! {
let feature = MLNPointFeature()
feature.coordinate = Polyline(route.legs[legIndex].steps[stepIndex].coordinates!.reversed()).coordinateFromStart(distance: instruction.distanceAlongStep)!
feature.coordinate = LineString(route.legs[legIndex].steps[stepIndex].coordinates!.reversed()).coordinateFromStart(distance: instruction.distanceAlongStep)!
feature.attributes = ["instruction": instruction.text]
features.append(feature)
}
Expand Down Expand Up @@ -1075,8 +1078,12 @@
Sets the camera directly over a series of coordinates.
*/
@objc public func setOverheadCameraView(from userLocation: CLLocationCoordinate2D, along coordinates: [CLLocationCoordinate2D], for bounds: UIEdgeInsets) {
assert(!coordinates.isEmpty, "must specify coordinates when setting overhead camera view")

self.isAnimatingToOverheadMode = true
let slicedLine = Polyline(coordinates).sliced(from: userLocation).coordinates
guard let slicedLine = LineString(coordinates).sliced(from: userLocation)?.coordinates else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

In which cases does LineString.sliced return nil in this case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same cause as my previous comment - sliced will now return nil rather than an empty LineString in the case that coordinates is empty.

The implications of this one are trickier though — it's possible that this might result in some change in behavior. My wager is that it's OK, since passing empty coordinates seems ill formed in the first place.

And in that case, our response is a no-op, rather than exploding or something scary.

I've just now added a debug assert to build confidence that we're not inadvertently hitting that code path.

What do you think?

return
}
let line = MLNPolyline(coordinates: slicedLine, count: UInt(slicedLine.count))

self.tracksUserCourse = false
Expand Down
Loading
Loading