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

Puck Location fix #2387

Merged
merged 1 commit into from
Jun 11, 2020
Merged

Puck Location fix #2387

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 8 additions & 4 deletions MapboxNavigation/NavigationMapView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,16 +340,20 @@ open class NavigationMapView: MGLMapView, UIGestureRecognizerDelegate {
return
}

let centerUserCourseView = { [weak self] in
guard let point = self?.convert(location.coordinate, toPointTo: self) else { return }
self?.userCourseView.center = point
}

if tracksUserCourse {
centerUserCourseView()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the smoothest implementation I found so far, to make user puck slide in place gracefully...

Copy link
Contributor

Choose a reason for hiding this comment

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

#2211 removed the line that set the view’s center. Can you check that it doesn’t introduce any regressions related to that PR? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Issue description and reproduction steps for #2211 are a bit cryptic for me, but as I understood, the issue was about incorrect puck positioning during navigation, together with zooming/panning/rotating map view. I checked that user puck is located right on the route regardless of camera position and switching to/from course tracking mode. Is that what #2211 was about?

Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately, I’m not super clear on it myself; I’m not entirely convinced that #2211 was the right fix for #2145, and it definitely wasn’t a fix for mapbox/mapbox-gl-native#15232 or mapbox/mapbox-gl-native#15233.

The main things to watch out for are #2145 (comment) and #2145 (comment).

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 could not reproduce symptoms described in linked comments...

Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand correctly, the Nav SDK is not using the Maps SDK user location annotation, which means it will not account for contentInset changes. I think this pr is trying to update the user location annotation considering the "insets".


let newCamera = camera ?? MGLMapCamera(lookingAtCenter: location.coordinate, altitude: altitude, pitch: 45, heading: location.course)
let function: CAMediaTimingFunction? = animated ? CAMediaTimingFunction(name: .linear) : nil
setCamera(newCamera, withDuration: duration, animationTimingFunction: function, completionHandler: nil)
} else {
// Animate course view updates in overview mode
UIView.animate(withDuration: duration, delay: 0, options: [.curveLinear], animations: { [weak self] in
guard let point = self?.convert(location.coordinate, toPointTo: self) else { return }
self?.userCourseView.center = point
})
UIView.animate(withDuration: duration, delay: 0, options: [.curveLinear], animations: centerUserCourseView)
}

userCourseView.update(location: location, pitch: self.camera.pitch, direction: direction, animated: animated, tracksUserCourse: tracksUserCourse)
Expand Down