Sticky course view tracking and persistent content inset #2047

frederoni · 2019-03-20T20:40:48Z

work in progress

Fixes UserCourseView should update instantaneously when user gestures cause camera changes #1731 - UserCourseView should update instantaneously
Fixes User course view lurches to the side when performing a maneuver #1949 - User course view lurches to the side
~~Fix CarPlay map fits to previewed route when returning to main map from custom template #2030 CarPlay map fitting~~ (already fixed in Fix route preview in CarPlay #2022)

This PR brings back the course tracking similar to the default in MGLMapView where the puck and camera are synchronized which is really noticeable when simulating faster than 1×.

cc @1ec5 @JThramer

frederoni · 2019-03-20T20:41:24Z

MapboxNavigation/CarPlayNavigationViewController.swift

+
+        if overviewing {
+            let padding = UIEdgeInsets(top: 40, left: 40, bottom: 40, right: 40)
+            contentInsets += padding


Max MBRouteLineWidthByZoomLevel should be accounted for.

1ec5 · 2019-03-20T21:26:47Z

MapboxNavigation/NavigationMapView.swift

-            UIView.animate(withDuration: duration, delay: 0, options: [.curveLinear, .beginFromCurrentState], animations: {
-                self.userCourseView?.center = self.convert(location.coordinate, toPointTo: self)
+            userCourseView?.center = userAnchorPoint
+        } else {


This reprises part of #1732. I take it #1732 (comment) is no longer a concern because the puck’s on-screen position is a fixed value now?

Exactly. The position is fixed unless frame by frame tracking is enabled which is needed when content insets or device orientation changes.

1ec5 · 2019-03-20T21:40:08Z

MapboxNavigation/RouteMapViewController.swift

+        if overviewing {
+            let courseView = mapView.userCourseView != nil ? mapView.userCourseView!.bounds.midX : 0
+            let routeLineWidths = MBRouteLineWidthByZoomLevel.compactMap { $0.value.constantValue as? Int }
+            let maxRouteLineWidth = CGFloat(routeLineWidths.max() ?? 0)


The resulting zoom level is the only one that matters, right?

1ec5 · 2019-03-22T17:04:27Z

#2030 was already fixed in #2022.

1ec5 · 2019-03-22T18:04:29Z

Apparently this issue regresses ~~#2030~~ fitting the camera to the route during preview, but it should be straightforward to isolate and undo that change.

1ec5 · 2019-03-22T18:48:16Z

Current state of this branch when previewing a route to a nearby destination using a search result in search history:

frederoni · 2019-03-22T20:58:30Z

The CarPlay preview is reverted and should behave exactly as before, .carPlayContentInsets(forOverviewing:) is only used when trackingUserCourse i set to true.

MapboxNavigation/CarPlayManager.swift

@@ -587,8 +587,7 @@ extension CarPlayManager: CPMapTemplateDelegate {
        topDownCamera.pitch = 0
        mapView.setCamera(topDownCamera, animated: false)

-        let padding = NavigationMapView.defaultPadding + mapView.safeArea
-        mapView.showcase([route], padding: padding)


1ec5

Things to add to the changelog:

Fixed an issue where the user puck moved around onscreen while tracking the user’s location.
Fixed an issue where the user puck briefly moved away from the route line as the user completed a turn.

1ec5 · 2019-03-22T23:32:24Z

MapboxNavigation/CarPlayNavigationViewController.swift

+
+        if !tracksUserCourse {
+            mapView.setContentInset(overviewContentInsets(), animated: false)
+            mapView.fit(to: navigationService.route, facing: 0, padding: .zero, animated: false)


Now if the user pans, disabling course tracking but without entering overview mode, the map will revert to course tracking once the navigation bar goes away. I guess that’s OK but somewhat unexpected.

You're right. We probably need a flag isOverviewing here similar to RouteMapViewController.
Panning around is currently not possible either with course tracking on/off nor overviewing (navigation bar popping in triggers viewSafeAreaInsetsDidChange() which in turn, fits the camera to the route).

@JThramer pointed out that this is actually pretty fortuitous. There isn’t a Resume or Recenter button like in NavigationViewController, and there’s no location tracking button like during the CarPlay browsing activity, so this code path is actually the most convenient way to get back into course tracking mode. Otherwise, the user has to tap the Overview button and tap the location tracking button after that.

1ec5 · 2019-03-22T23:34:39Z

MapboxNavigation/NavigationMapView.swift

+        currentCamera.pitch = 0
+        currentCamera.heading = 0
+
+        let newCamera = camera(currentCamera, fitting: line, edgePadding: padding)


This is equivalent to calling cameraThatFits(_:direction:edgePadding:), since we’re forcing the pitch to be 0° anyways.

1ec5 · 2019-03-22T23:37:55Z

MapboxNavigation/RouteMapViewController.swift

+            insets += NavigationMapView.courseViewMinimumInsets
+
+            let routeLineWidths = MBRouteLineWidthByZoomLevel.compactMap { $0.value.constantValue as? Int }
+            insets += UIEdgeInsets(floatLiteral: Double(routeLineWidths.max() ?? 0))


👍 As noted in #2047 (comment), this is rather conservative considering that only one zoom level is active at a given time. But we don‘t have ready access to the evaluated expression (unlike if we were using GL JS), and this method also gets called before calculating a camera rather than after, so choosing the right width isn’t worth the hassle. In practice, it would only be a few extra points on each side at most.

…nt insets

frederoni added the ⚠️ DO NOT MERGE PR should not be merged! label Mar 20, 2019

frederoni commented Mar 20, 2019

View reviewed changes

1ec5 reviewed Mar 20, 2019

View reviewed changes

datwelk mentioned this pull request Mar 21, 2019

User puck not in sync with route at high simulation speeds #1920

Closed

akitchen added this to the v0.30.0 milestone Mar 21, 2019

frederoni force-pushed the fred/course-view-edge-pad branch from a6edd1e to e79599c Compare March 22, 2019 20:46

frederoni commented Mar 22, 2019

View reviewed changes

frederoni force-pushed the fred/course-view-edge-pad branch 3 times, most recently from e01d000 to e9450fc Compare March 22, 2019 21:38

1ec5 approved these changes Mar 22, 2019

View reviewed changes

frederoni added 8 commits March 23, 2019 00:48

Fix sticky course view and consistency between edge padding and conte…

9ffa3db

…nt insets

Drop test code

7353929

Clean up content insets calculation

a80b7cf

Temporary CarPlay overview tweaks

124f021

Revert CarPlay overview changes

5e259aa

Don't compensate for the vanishing point in overview mode

502b0e3

Remove dead code

cfbf0e6

Fix duplicated content insets in RouteMapViewController

0326e45

frederoni force-pushed the fred/course-view-edge-pad branch from f6e640b to 0326e45 Compare March 22, 2019 23:49

JThramer removed the ⚠️ DO NOT MERGE PR should not be merged! label Mar 23, 2019

Update CHANGELOG.md

59f5146

frederoni merged commit 944a611 into master Mar 23, 2019

akitchen mentioned this pull request Mar 25, 2019

User puck "yo-yo" effect when simulating navigation in CarPlay due to multiple location managers #1941

Closed

1ec5 mentioned this pull request Aug 14, 2019

Simplify anchor point calculation and puck centering #2211

Merged

1ec5 deleted the fred/course-view-edge-pad branch December 5, 2019 18:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sticky course view tracking and persistent content inset #2047

Sticky course view tracking and persistent content inset #2047

frederoni commented Mar 20, 2019 •

edited by 1ec5

Loading

frederoni Mar 20, 2019

1ec5 Mar 20, 2019

frederoni Mar 22, 2019

1ec5 Mar 20, 2019

1ec5 commented Mar 22, 2019

1ec5 commented Mar 22, 2019 •

edited

Loading

1ec5 commented Mar 22, 2019 •

edited

Loading

frederoni commented Mar 22, 2019 •

edited

Loading

This comment was marked as resolved.

1ec5 left a comment

1ec5 Mar 22, 2019

frederoni Mar 22, 2019

1ec5 Mar 23, 2019

1ec5 Mar 22, 2019

1ec5 Mar 22, 2019

Sticky course view tracking and persistent content inset #2047

Sticky course view tracking and persistent content inset #2047

Conversation

frederoni commented Mar 20, 2019 • edited by 1ec5 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 commented Mar 22, 2019

1ec5 commented Mar 22, 2019 • edited Loading

1ec5 commented Mar 22, 2019 • edited Loading

frederoni commented Mar 22, 2019 • edited Loading

This comment was marked as resolved.

1ec5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frederoni commented Mar 20, 2019 •

edited by 1ec5

Loading

1ec5 commented Mar 22, 2019 •

edited

Loading

1ec5 commented Mar 22, 2019 •

edited

Loading

frederoni commented Mar 22, 2019 •

edited

Loading