[ios] Add permanent user heading indicator #9886

friedbunny · 2017-08-29T23:31:59Z

This is an WIP towards fixing #5523, adding a permanent user heading indicator (visible outside of heading tracking mode) and refactoring the user location annotation a bit.

To-do

/cc @1ec5 @fabian-guerra @jmkiley

friedbunny · 2017-08-30T23:12:56Z

Preliminary design:

Note sure if it would look better overlapping the white border or if it should stay outside. 🤔

/cc @mayagao

fabian-guerra · 2017-08-31T13:59:58Z

Is it possible see the contrast between the arrow heading <-> beam indicator with Mapbox Streets and Dark styles?

friedbunny · 2017-08-31T21:58:56Z

e0b27fe adds a 1pt white stroke to the arrow and moves it on top of the outer white dot layer. I think this improves legibility with darker tint colors (and it’s kinda’ cute). 🐦

Still need to test this with different pixel densities and tint colors. Will also look at extending the shadow path to include the arrow.

1ec5

This is nice. I kind of want to borrow it for macOS SDK’s -[MGLCompassCell drawKnob:]. 😄

1ec5 · 2017-09-01T08:01:09Z

platform/ios/src/MGLMapView.h

+
+ The default value of this property is `NO`.
+ */
+@property (nonatomic, assign) BOOL showsUserHeadingIndicator;


Should this property be inspectable? What happens if this property is set but showsUserLocation is not?

Should this property be inspectable?

Sure, that would be useful — I’ll do it.

What happens if this property is set but showsUserLocation is not?

showsUserHeadingIndicator won’t do anything without showsUserLocation=YES — it doesn’t cause the user location annotation to appear. I considered having showsUserHeadingIndicator also implicitly enable showsUserLocation (the way our tracking modes do), but balked because it seemed a little too magical/indirect. Considering it again now, I could probably go either way. What do you think?

In any case, I’ll add documentation for this.

Made this inspectable in 776a1c3 — renamed it for IB for disambiguation.

fa692f8: now with less truncation.

friedbunny · 2017-09-06T23:18:40Z

I’ve rebased and reorganized this into digestible commits. I’m relatively happy with the state it’s in and would appreciate review from y’all. 🙇

A couple notes on the heading filter and disabled implicit animation:

The heading filter has been reduced to 1° (the default) from 5°, which smooths the rotation of the map and indicator by receiving/handling more heading updates.
Performance (in terms of FPS) in heading tracking mode should be essentially the same as before, as the heading indicator no longer has implicit animations bogging things down.
Perceived performance should be improved, as we no longer wait 5° between map updates (which could be jerky).
One downside: without the implicit animation, large heading changes will cause the indicator to jump. Ideally we would animate large changes, but I’d prefer to leave that as tail work.

/cc @fabian-guerra @1ec5 @frederoni

1ec5

It’s great to see MGLFaux3DUserLocationAnnotationView refactored this way, and the arrow’s design is a nice touch. Besides the relatively minor feedback below, would you do the honors and update the iOS changelog?

1ec5 · 2017-09-07T00:09:17Z

platform/ios/app/MBXViewController.m

    } else if (self.showZoomLevelEnabled) {
-        self.hudLabel.text = [NSString stringWithFormat:@" Zoom: %.2f", self.mapView.zoomLevel];
+        hudString = [NSString stringWithFormat:@"%.2f ∕ ↕\U0000FE0E%.f° ∕ %.f°", self.mapView.zoomLevel, self.mapView.camera.pitch, self.mapView.direction];


Verified that this is U+00B0 DEGREE SIGN. ✅

BTW, this ends up looking like this in the label:

14.12 ∕ ↕︎45° ∕ 280°

... thanks to the good ole variation selector, which negates emoji conversion.

1ec5 · 2017-09-07T20:06:02Z

platform/ios/src/MGLFaux3DUserLocationAnnotationView.h

+const CGFloat MGLUserLocationAnnotationHaloSize = 115.0;
+
+const CGFloat MGLUserLocationAnnotationPuckSize = 45.0;
+const CGFloat MGLUserLocationAnnotationArrowSize = MGLUserLocationAnnotationPuckSize * 0.6;


Are these constants used anywhere outside of MGLFaux3DUserLocationAnnotationView.m? If not, I think they can stay in that file.

They were during the initial refactor, but I ended up trying to avoid using these outside of MGLFaux3DUserLocationAnnotationView.m. I’ll see about moving these back to there.

MGLUserLocationHeadingBeamLayer still needs MGLUserLocationAnnotationHaloSize, which unfortunately can’t be easily derived from the user location annotation itself.

1ec5 · 2017-09-07T20:08:59Z

platform/ios/src/MGLFaux3DUserLocationAnnotationView.m

+            CGFloat rotation = -MGLRadiansFromDegrees(self.mapView.direction - self.userLocation.heading.trueHeading);
+
+            // Don't rotate if the change is imperceptible (arbitrarily chosen as: 0.01 radians/~0.6°).
+            if (fabs(rotation) > 0.01)


Hoist this up top as a constant. Worth noting that this tolerance is much smaller than the other angular tolerance, MGLToleranceForSnappingToNorth, but I think that’s OK because the unrotation animation is meant to be perceptible.

The default is 1.

Rename it to `showsHeading` to fit in the tiny label column of the utilities panel.

…ndicator

The update steps for the heading indicator are typically small, so animations tend to pile up and cause performance issues. Disabling actions is a slight regression when it comes to large steps (they're not animated now, where they previously were) and this should eventually be addressed. Also consistently use provided API for disabling CATransaction actions.

friedbunny · 2017-09-08T00:32:04Z

Tests are failing on the release branch because of egl-node6-clang39-debug builds on Travis, such as this. Are you aware of any changes in master that we can cherry-pick to fix this, @brunoabinader @bsudekum?

brunoabinader · 2017-09-11T13:46:58Z

Not really @friedbunny 😞 this is the first time I see this failure.

friedbunny added feature Google Maps parity For feature parity with the Google Maps SDK for Android or iOS iOS Mapbox Maps SDK for iOS MapKit parity For feature parity with MapKit on iOS or macOS refactor labels Aug 29, 2017

friedbunny added this to the ios-v3.6.3 milestone Aug 29, 2017

friedbunny self-assigned this Aug 29, 2017

1ec5 reviewed Sep 1, 2017

View reviewed changes

friedbunny force-pushed the fb-permaheading branch from 776a1c3 to fa692f8 Compare September 1, 2017 19:32

[ios] Improve iosapp's zoom debug label

cb7eb5c

friedbunny force-pushed the fb-permaheading branch from fa692f8 to 7c7ced9 Compare September 6, 2017 22:19

[ios] Refactor user location heading indicator into its own class

c670844

friedbunny force-pushed the fb-permaheading branch 2 times, most recently from 6dcd9a8 to cc81649 Compare September 6, 2017 23:07

friedbunny requested review from 1ec5, fabian-guerra and frederoni September 6, 2017 23:09

friedbunny removed the MapKit parity For feature parity with MapKit on iOS or macOS label Sep 6, 2017

1ec5 approved these changes Sep 7, 2017

View reviewed changes

friedbunny changed the title ~~[ios] WIP: Add permanent user heading indicator~~ [ios] Add permanent user heading indicator Sep 7, 2017

friedbunny added 6 commits September 7, 2017 19:23

[ios] Introduce MGLUserLocationHeadingArrowLayer class

5db97ed

[ios] Set a heading indicator update threshold

c35b1cd

[ios] Introduce MGLUserLocationHeadingIndicator protocol

99616e6

[ios] Add setShowsUserHeadingIndicator and validateUserHeadingUpdating

218c720

[ios] Change user location heading filter to 1°

e1ff826

The default is 1.

[ios] Make user heading indicator property IBInspectable

63250ba

Rename it to `showsHeading` to fit in the tiny label column of the utilities panel.

friedbunny added 3 commits September 7, 2017 19:23

[ios] Implicitly enable showsUserLocation when enabling the heading i…

cb1309a

…ndicator

[ios] Update changelog for MGLMapView.showsUserHeadingIndicator

efe9928

friedbunny force-pushed the fb-permaheading branch from cc81649 to efe9928 Compare September 7, 2017 23:49

friedbunny mentioned this pull request Sep 8, 2017

egl-node6-clang39 builds are failing on the v3.6.x release branch #9940

Closed

friedbunny merged commit efe9928 into release-ios-v3.6.0-android-v5.1.0 Sep 11, 2017

friedbunny deleted the fb-permaheading branch September 11, 2017 21:13

This was referenced Sep 11, 2017

User dot should show minimal heading indicator outside of tracking mode #5523

Closed

Show/hide user heading indicator based on accuracy #9992

Open

1ec5 mentioned this pull request Sep 28, 2017

Define user dot constants in implementation file #10087

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ios] Add permanent user heading indicator #9886

[ios] Add permanent user heading indicator #9886

friedbunny commented Aug 29, 2017 •

edited

Loading

friedbunny commented Aug 30, 2017 •

edited

Loading

fabian-guerra commented Aug 31, 2017

friedbunny commented Aug 31, 2017 •

edited

Loading

1ec5 left a comment

1ec5 Sep 1, 2017

friedbunny Sep 1, 2017 •

edited

Loading

friedbunny Sep 1, 2017 •

edited

Loading

friedbunny Sep 1, 2017

friedbunny commented Sep 6, 2017 •

edited

Loading

1ec5 left a comment

1ec5 Sep 7, 2017

friedbunny Sep 7, 2017

1ec5 Sep 7, 2017

friedbunny Sep 7, 2017

friedbunny Sep 7, 2017 •

edited

Loading

1ec5 Sep 7, 2017

friedbunny commented Sep 8, 2017

brunoabinader commented Sep 11, 2017

[ios] Add permanent user heading indicator #9886

[ios] Add permanent user heading indicator #9886

Conversation

friedbunny commented Aug 29, 2017 • edited Loading

To-do

friedbunny commented Aug 30, 2017 • edited Loading

fabian-guerra commented Aug 31, 2017

friedbunny commented Aug 31, 2017 • edited Loading

1ec5 left a comment

Choose a reason for hiding this comment

1ec5 Sep 1, 2017

Choose a reason for hiding this comment

friedbunny Sep 1, 2017 • edited Loading

Choose a reason for hiding this comment

friedbunny Sep 1, 2017 • edited Loading

Choose a reason for hiding this comment

friedbunny Sep 1, 2017

Choose a reason for hiding this comment

friedbunny commented Sep 6, 2017 • edited Loading

1ec5 left a comment

Choose a reason for hiding this comment

1ec5 Sep 7, 2017

Choose a reason for hiding this comment

friedbunny Sep 7, 2017

Choose a reason for hiding this comment

1ec5 Sep 7, 2017

Choose a reason for hiding this comment

friedbunny Sep 7, 2017

Choose a reason for hiding this comment

friedbunny Sep 7, 2017 • edited Loading

Choose a reason for hiding this comment

1ec5 Sep 7, 2017

Choose a reason for hiding this comment

friedbunny commented Sep 8, 2017

brunoabinader commented Sep 11, 2017

friedbunny commented Aug 29, 2017 •

edited

Loading

friedbunny commented Aug 30, 2017 •

edited

Loading

friedbunny commented Aug 31, 2017 •

edited

Loading

friedbunny Sep 1, 2017 •

edited

Loading

friedbunny Sep 1, 2017 •

edited

Loading

friedbunny commented Sep 6, 2017 •

edited

Loading

friedbunny Sep 7, 2017 •

edited

Loading