Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Various user tracking mode corrections #3680

Merged
merged 8 commits into from
Jan 27, 2016
Merged

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Jan 24, 2016

This PR fixes a variety of minor issues around user tracking mode and camera animation that were discovered while attempting to make use of the user dot alignment option introduced in #3589.

I’ve also added a “targeted” user tracking mode: when following the user’s course, you can specify a targetCoordinate. As the user nears the target, the map continually recalculates the viewport to fit both the user and the target in a vertical orientation.

Fixes #1145.

/cc @friedbunny @bsudekum

@1ec5 1ec5 added bug iOS Mapbox Maps SDK for iOS labels Jan 24, 2016
@1ec5 1ec5 added this to the ios-v3.1.0 milestone Jan 24, 2016
@friedbunny
Copy link
Contributor

Changing/enabling userTrackingMode with a non-center vertical alignment is still animating to the center position on the map, while keeping the dot at the expected screen location (bottom, in this case) but wrong geographic location... until the next location update(?).

ezgif com-optimize

@1ec5
Copy link
Contributor Author

1ec5 commented Jan 25, 2016

Changing/enabling userTrackingMode with a non-center vertical alignment is still animating to the center position on the map, while keeping the dot at the expected screen location (bottom, in this case) but wrong geographic location... until the next location update(?).

Fixed in 7b820ab. Indeed, the initial animation (which uses flyTo, unlike incremental updates) failed to account for the vertical alignment.

@1ec5 1ec5 self-assigned this Jan 25, 2016
@mb12
Copy link

mb12 commented Jan 25, 2016

The dot seems to be lagging behind the map a lot upon panning.

@1ec5
Copy link
Contributor Author

1ec5 commented Jan 25, 2016

@mb12, does it appear to be worse than in 3.0.1? I don’t know of any changes in 3.1.0-pre.1 that would’ve exacerbated the issue being tracked in #1125.

3.1.0-pre.1 and this PR only affect the user dot in user tracking mode. In this mode, the dot isn’t even supposed to move relative to the screen anymore. #1582 would affect the user dot outside user tracking mode; the remaining work in that PR is to eliminate the lagging you’re experiencing.

@mb12
Copy link

mb12 commented Jan 25, 2016

I haven't tried the latest update yet. I will try and let you know.
The lag in the attached animation seemed a lot more than what I've noticed on the device(one from a really old repo). That's why I mentioned it.

@1ec5
Copy link
Contributor Author

1ec5 commented Jan 25, 2016

Let’s continue tracking the lag issue in #3683.

@friedbunny
Copy link
Contributor

Rotating the device while in heading tracking mode, with bottom-alignment, causes the map to move incorrectly:

ezgif com-optimize-2

@1ec5
Copy link
Contributor Author

1ec5 commented Jan 25, 2016

That was fixed in fb8d336. I just forgot to push. 😉

@friedbunny
Copy link
Contributor

That was fixed in fb8d336.

Yup!

Now I've got another one for ya': rotating the screen (portrait↔landscape) with non-center alignment causes the user dot location to shift incorrectly, then animate back to the correct location.

@1ec5
Copy link
Contributor Author

1ec5 commented Jan 26, 2016

Something similar also happens when the user puck is not shifted in course tracking mode. Probably a regression from the latest refactoring.

@friedbunny
Copy link
Contributor

rotating the screen (portrait↔landscape) with non-center alignment causes the user dot location to shift incorrectly, then animate back to the correct location.

I'm still seeing this on the second rotation; i.e., portrait → landscape (👍) → portrait (👎, dot shifts geographically).

@1ec5
Copy link
Contributor Author

1ec5 commented Jan 26, 2016

portrait → landscape (👍) → portrait (👎, dot shifts geographically).

Fixed in b5f1b9e1cf93f6afc08d15721347746ff69e588e.

The Travis failure is unrelated to these changes, by the way.

@friedbunny
Copy link
Contributor

I'm starting to get into edge cases, but animated setting of non-center alignment while also turning on heading tracking will result in some slightly geographically incorrect intermediate animations until heading mode completely engages.

diff --git a/ios/app/MBXViewController.mm b/ios/app/MBXViewController.mm
index fddab78..7f3e7c9 100644
--- a/ios/app/MBXViewController.mm
+++ b/ios/app/MBXViewController.mm
@@ -442,9 +446,11 @@ static const CLLocationCoordinate2D WorldTourDestinations[] = {
     switch (self.mapView.userTrackingMode) {
         case MGLUserTrackingModeNone:
             nextMode = MGLUserTrackingModeFollow;
+            [self.mapView setUserLocationVerticalAlignment:MGLAnnotationVerticalAlignmentCenter animated:YES];
             break;
         case MGLUserTrackingModeFollow:
             nextMode = MGLUserTrackingModeFollowWithHeading;
+            [self.mapView setUserLocationVerticalAlignment:MGLAnnotationVerticalAlignmentBottom animated:YES];
             break;
         case MGLUserTrackingModeFollowWithHeading:
             nextMode = MGLUserTrackingModeFollowWithCourse;

@friedbunny
Copy link
Contributor

When initializing with some tracking mode enabled (but no explicit .showsUserlocation), the user dot won't be shown until the map is interacted with or the camera changes.

It's necessary to disable [self restoreState:nil] in iosapp for this to occur.

@1ec5
Copy link
Contributor Author

1ec5 commented Jan 27, 2016

animated setting of non-center alignment while also turning on heading tracking will result in some slightly geographically incorrect intermediate animations until heading mode completely engages.

Either I was unable to reproduce this, or the errors were very minor. I’m comfortable punting on this issue. If a developer needs both the user tracking mode and the alignment to be animated, the alignment can be set in -[MGLMapViewDelegate mapView:didChangeUserTrackingMode:animated:].

When initializing with some tracking mode enabled (but no explicit .showsUserlocation), the user dot won't be shown until the map is interacted with or the camera changes.

Fixed in fb7de377a280a68c0a3b29987d591b82fab62b48.

@friedbunny
Copy link
Contributor

animated setting of non-center alignment while also turning on heading tracking will result in some slightly geographically incorrect intermediate animations until heading mode completely engages.

I'm not seeing this anymore — the dot moves to the correct position, then rotates to the correct heading. 🤷

@1ec5
Copy link
Contributor Author

1ec5 commented Jan 27, 2016

c087b485e2d38e6834e796fd55a8f7a4a97b5db7 fixes the tilt gesture to respect the content insets and a non-center-aligned user dot. The gesture feels a lot better now. However, if you tilt the map during targeted course tracking mode, the map iteratively zeros in on the correct bounds. It’s a pretty jarring experience, one that will only be addressed by fixing #2259.

Setting the user tracking mode without animation now works. Previously, it kept the user dot from ever updating.

Just as a zoom gesture no longer kicks the user out of user tracking mode, programmatically zooming shouldn’t either.

Setting a camera with an invalid center coordinate no longer attempts to change the center coordinate but still changes any other valid properties.

Made animation to new user dot vertical alignment optional.
Account for the user dot’s alignment when flying to the initial user location. Previously, during incremental location updates, we relied on a hack in which the edge insets resulted in a 0×0 viewport, forcing the center upward or downward. But flyTo() relies on the viewport’s size to control the trajectory. So instead, all location updates now use a correctly-sized viewport centered on the offset user dot.

Pushed the user dot farther away from the view edges.

Programmatic rotation and compass rotation are now centered on the user dot even if it’s aligned at the top or bottom. Previously, pressing the compass would trigger a rotation centered on the center of the view.
Also fixed top- and bottom-aligned user dot center points to account for the content inset’s offset within the map view bounds.
When a targetCoordinate is specified in course tracking mode, the map automatically resizes the viewport to show both the user puck and the target, one at the top and the other at the bottom. The user puck now rotates its arrow in the course direction, no longer assuming that the viewport is facing the same way as the course.
Broke out -locationManager:didUpdateLocations:animated: into multiple self-contained methods, simplifying the logic considerably.

Eliminated a jump after device rotation in non-centered tracking mode.
@1ec5 1ec5 force-pushed the 1ec5-tracking-non-animated branch from c087b48 to a16c210 Compare January 27, 2016 01:21
Entering user tracking mode at launch now zooms in and shows the user dot or user puck. The user dot’s heading indicator now points in the correct direction during the animation to the initial location. Course changes are reflected immediately even in the absence of location changes.

Fixes #1145.
The tilt gesture on both iOS and OS X now respects the content insets. On iOS, in user tracking mode, it additionally respects the user dot’s position if it’s aligned to the top or bottom of the view.
@1ec5 1ec5 force-pushed the 1ec5-tracking-non-animated branch from a2b7a98 to 8a23bdc Compare January 27, 2016 02:49
@1ec5 1ec5 merged commit a5283b5 into master Jan 27, 2016
@1ec5 1ec5 removed the ready label Jan 27, 2016
@1ec5 1ec5 deleted the 1ec5-tracking-non-animated branch January 27, 2016 07:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug iOS Mapbox Maps SDK for iOS refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to turn on user location tracking immediately at startup
3 participants