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

Jump to bottom

Fixed Flying Puck issue #1732

Closed
wants to merge 1 commit into from
Closed

Fixed Flying Puck issue #1732

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
11 changes: 9 additions & 2 deletions MapboxNavigation/NavigationMapView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,9 +337,16 @@ open class NavigationMapView: MGLMapView, UIGestureRecognizerDelegate {
let padding = UIEdgeInsets(top: point.y, left: point.x, bottom: bounds.height - point.y, right: bounds.width - point.x)
let newCamera = camera ?? MGLMapCamera(lookingAtCenter: location.coordinate, fromDistance: altitude, pitch: 45, heading: location.course)
let function: CAMediaTimingFunction? = animated ? CAMediaTimingFunction(name: kCAMediaTimingFunctionLinear) : nil
setCamera(newCamera, withDuration: duration, animationTimingFunction: function, edgePadding: padding, completionHandler: nil)
setCamera(newCamera, withDuration: duration, animationTimingFunction: function, edgePadding: padding) { [weak self] in
guard let strongSelf = self else { return }
if strongSelf.userAnchorPoint != strongSelf.userCourseView?.center ?? strongSelf.userAnchorPoint {
UIView.animate(withDuration: 0, delay: 0, options: [.curveLinear, .beginFromCurrentState], animations: {
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’m reading this correctly, we’re chaining animations together, snapping the course view back to an accurate location after an initial animation. But isn’t it that initial animation that looks incorrect?

Copy link
Contributor

Choose a reason for hiding this comment

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

As suggested in the ticket (#1731), I think setting the duration on the outer animation closure to 0 when a user is interacting with the map is a better idea.

Copy link
Contributor

@frederoni frederoni Sep 26, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This patch seems to be fixing the turntable part in #1731

turntable.patch 
diff --git a/MapboxNavigation/NavigationMapView.swift b/MapboxNavigation/NavigationMapView.swift
index 3d2b9ea6..ceaf87da 100644
--- a/MapboxNavigation/NavigationMapView.swift
+++ b/MapboxNavigation/NavigationMapView.swift
@@ -411,6 +411,16 @@ open class NavigationMapView: MGLMapView, UIGestureRecognizerDelegate {
             guard let location = userLocationForCourseTracking else { return }
             userCourseView?.layer.removeAllAnimations()
             userCourseView?.center = convert(location.coordinate, toPointTo: self)
+            
+            if let view = userCourseView as? UserCourseView,
+                let location = userLocationForCourseTracking {
+                
+                view.update?(location: location,
+                             pitch: self.camera.pitch,
+                             direction: self.direction,
+                             animated: false,
+                             tracksUserCourse: self.tracksUserCourse)
+            }
         }
     }

strongSelf.userCourseView?.center = strongSelf.convert(location.coordinate, toPointTo: strongSelf)
})
}
}
}
if !tracksUserCourse || userAnchorPoint != userCourseView?.center ?? userAnchorPoint {
if !tracksUserCourse {
Copy link
Contributor

Choose a reason for hiding this comment

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

This change effectively reverts #1653. We’ll need to verify the results in CarPlay.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just tried this in a CarPlay unit – it’s worse than what I saw in #1731 (comment) on the simulator for some reason. The main map has the puck shifted upwards by about a third of the screen, and it isn’t possible to show the route at any time (whether in preview, turn-by-turn, or overview mode).

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