-
Notifications
You must be signed in to change notification settings - Fork 310
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 UserCourseView instantaneously #1838
Conversation
to a gesture recognizer
@@ -225,6 +225,11 @@ open class NavigationMapView: MGLMapView, UIGestureRecognizerDelegate { | |||
addGestureRecognizer(mapTapGesture) | |||
} | |||
|
|||
open override func layoutMarginsDidChange() { | |||
super.layoutMarginsDidChange() | |||
enableFrameByFrameCourseViewTracking(for: 3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would preferably avoid this frame by frame tracking but using updateCourseTracking(location:camera:animated:false)
did not mitigate the lag/animation. cc @1ec5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps there’s a race condition in which this method gets called before the map view updates its center coordinate to reflect the margin change, so that by the time we convert the user location to a center point within the (non)animation block, the center point still reflects the old margins.
mapbox-navigation-ios/MapboxNavigation/NavigationMapView.swift
Lines 332 to 334 in a0e5dbc
UIView.animate(withDuration: duration, delay: 0, options: [.curveLinear, .beginFromCurrentState], animations: { | |
self.userCourseView?.center = self.convert(location.coordinate, toPointTo: self) | |
}) |
Temporarily enabling frame-by-frame course tracking ensures that the view moves as soon as the map adjusts to the new content insets (since that requires the map view to render another frame):
mapbox-navigation-ios/MapboxNavigation/NavigationMapView.swift
Lines 261 to 264 in a0e5dbc
guard shouldPositionCourseViewFrameByFrame else { return } | |
guard let location = userLocationForCourseTracking else { return } | |
userCourseView?.center = convert(location.coordinate, toPointTo: self) |
Temporarily reenabling frame-by-frame course tracking isn’t the end of the world, though if it really comes down to this race condition, then I’d think a shorter duration would still be sufficient to work around it.
ce7b1bd
to
5d0a34f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested changelog entry:
Fixed an issue causing
UserCourseView
to lag behindNavigationMapView
whenever the map’s camera or content insets change significantly.
@@ -225,6 +225,11 @@ open class NavigationMapView: MGLMapView, UIGestureRecognizerDelegate { | |||
addGestureRecognizer(mapTapGesture) | |||
} | |||
|
|||
open override func layoutMarginsDidChange() { | |||
super.layoutMarginsDidChange() | |||
enableFrameByFrameCourseViewTracking(for: 3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps there’s a race condition in which this method gets called before the map view updates its center coordinate to reflect the margin change, so that by the time we convert the user location to a center point within the (non)animation block, the center point still reflects the old margins.
mapbox-navigation-ios/MapboxNavigation/NavigationMapView.swift
Lines 332 to 334 in a0e5dbc
UIView.animate(withDuration: duration, delay: 0, options: [.curveLinear, .beginFromCurrentState], animations: { | |
self.userCourseView?.center = self.convert(location.coordinate, toPointTo: self) | |
}) |
Temporarily enabling frame-by-frame course tracking ensures that the view moves as soon as the map adjusts to the new content insets (since that requires the map view to render another frame):
mapbox-navigation-ios/MapboxNavigation/NavigationMapView.swift
Lines 261 to 264 in a0e5dbc
guard shouldPositionCourseViewFrameByFrame else { return } | |
guard let location = userLocationForCourseTracking else { return } | |
userCourseView?.center = convert(location.coordinate, toPointTo: self) |
Temporarily reenabling frame-by-frame course tracking isn’t the end of the world, though if it really comes down to this race condition, then I’d think a shorter duration would still be sufficient to work around it.
# Conflicts: # CHANGELOG.md
Fixes #1731
cc @1ec5 @vincethecoder @JThramer