User course view lurches to the side when performing a maneuver

[1ec5]

NavigationMapView.shouldPositionCourseViewFrameByFrame keeps the user course view (puck) constrained to the map on each frame, but it only ever seems to be enabled in response to external changes to the map view rather than internal changes due to location updates. When this property was originally added in #402, the idea was that the map would lasso the puck and force-synchronize it with the map on each frame, rather than allowing it to roam free using its normal 1-second animation, in response to location updates. At some point, this distinction was lost.

As a result of this omission, as the route takes a right turn, we send the puck to the right based on where the next location along the cross street is currently displayed on the map. That movement is animated with a duration of 1 second. But at the same time, we kick off a 1-second-long camera animation that rotates the map to look down the cross street. The effect is that the puck appears to lurch to the right before swinging back to the center of the map to meet the route line. This effect is exacerbated by the route simulation feature’s speedup factor, if specified.

At a glance, the fix appears to be simple: we already correctly synchronize the puck to the map when NavigationMapView.shouldPositionCourseViewFrameByFrame is enabled, but we need to enable that property via NavigationMapView.enableFrameByFrameCourseViewTracking(for:) when approaching a maneuver. There’s some existing logic along these lines, but it only adjusts the map view’s frame rate rather than keeping shouldPositionCourseViewFrameByFrame enabled:

mapbox-navigation-ios/MapboxNavigation/NavigationMapView.swift

Line 273 in 9e83261

userCourseView?.center = convert(location.coordinate, toPointTo: self)

mapbox-navigation-ios/MapboxNavigation/NavigationMapView.swift

Lines 295 to 296 in 9e83261

	} else if durationUntilNextManeuver > FrameIntervalOptions.durationUntilNextManeuver &&
	durationSincePreviousManeuver > FrameIntervalOptions.durationSincePreviousManeuver {

/cc @mapbox/navigation-ios

[akitchen]

This suggested solution makes sense at tight maneuvers, but per team conversation this might not make sense on curvy roads where we sometimes see a similar issue. We're currently not sure that this suggested solution fixes the root problem.

Next steps are for @frederoni & @JThramer to dig into the problem and see if we have a better idea of the ideal solution for the root problem, for example incorporating additional control points on an adaptive basis.

[1ec5]

In other words, shouldPositionCourseViewFrameByFrame would probably address the issue at maneuvers, but only if that setting can force the preferred map frame rate to be 60 fps – as it does by default, I think.

We also see similar behavior along curvy roads, and keeping that setting on wherever the road isn’t perfectly straight would result in poor performance. So @frederoni proposed getting interpolated locations (perhaps using navigation-native or Turf), so that we aren’t so dependent on the granularity with which the road has been mapped. That would also address #1908, where we spin the camera the wrong way when making a fast sharp turn – frame-by-frame positioning itself won’t help because the shortest path around a circle still goes the wrong way.

At its core, the problem here is about screen coordinates rather than geographic coordinates, so any approach based on CLLocationCoordinate2Ds will be limited in terms of how smooth we can make the animation. Ideally, we’d come up with something similar to how the map SDK’s view-backed annotations work – view-backed annotations are essentially always positioned frame by frame, with no UIView animation when the map camera is changing. Even if shouldPositionCourseViewFrameByFrame doesn’t override the map’s frame rate, maybe that’s fine as long as the puck and map are consistent with one another.

[1ec5]

shouldPositionCourseViewFrameByFrame conflates two different mechanisms. We want to position the course view frame by frame basically always:

mapbox-navigation-ios/MapboxNavigation/NavigationMapView.swift

Lines 270 to 273 in 64f07b7

	guard shouldPositionCourseViewFrameByFrame else { return }
	guard let location = userLocationForCourseTracking else { return }
	userCourseView?.center = convert(location.coordinate, toPointTo: self)

But we don’t want to affect the map’s frame rate in doing so:

mapbox-navigation-ios/MapboxNavigation/NavigationMapView.swift

Lines 292 to 297 in 64f07b7

	} else if let upcomingStep = routeProgress.currentLegProgress.upcomingStep,
	upcomingStep.maneuverDirection == .straightAhead || upcomingStep.maneuverDirection == .slightLeft || upcomingStep.maneuverDirection == .slightRight {
	preferredFramesPerSecond = shouldPositionCourseViewFrameByFrame ? FrameIntervalOptions.defaultFramesPerSecond : minimumFramesPerSecond
	} else if durationUntilNextManeuver > FrameIntervalOptions.durationUntilNextManeuver &&
	durationSincePreviousManeuver > FrameIntervalOptions.durationSincePreviousManeuver {
	preferredFramesPerSecond = shouldPositionCourseViewFrameByFrame ? FrameIntervalOptions.defaultFramesPerSecond : minimumFramesPerSecond

Similar to #1731, we never want UIView animation when the camera changes due to course tracking. It would be ideal if we could just do this whenever the course changes:

mapbox-navigation-ios/MapboxNavigation/NavigationMapView.swift

Line 273 in 64f07b7

userCourseView?.center = convert(location.coordinate, toPointTo: self)

But unfortunately, due to the lack of concurrent animations in the map SDK, that would preempt any desirable animations used to interpolate between location updates: mapbox/mapbox-gl-native#3625 (comment).

Interpolating locations won’t actually help with the specific scenario above, at a standard turn, because the location is more or less atomic. What’s changing is the course, so we’d need to interpolate course updates. UIView animation would still be a fallback for when there isn’t enough course interpolation.

[frederoni]

I did a quick route following test with UIView animations disabled and positioning the course view on a frame by frame basis at all time including puck transformations.

Recording

The interpolation is a little jumpy once per second and the unsynchronized transformation makes the puck look like a motorcycle leaning in the corners, but the puck never leaves the route line by a single pixel.

Similar to #1731, we never want UIView animation when the camera changes due to course tracking. It would be ideal if we could just do this whenever the course changes:

This could potentially simplify things but I think the camera and puck would look very static.