NavigationMapRoute 2.0 #1387

danesfeder · 2018-10-03T17:43:12Z

Closes #704 Fixes #1512

This PR aims to refactor the monstrous NavigationMapRoute and solve the issues of the route freezing the UI thread when in the process of drawing new routes.

Part 1 is processing traffic data on a background thread. Part 2 is going to be optimizing how we update the map source after we have the data we need.

TODO:

Look into Running real machine, The console prints the following information #1512

cc @tobrun @LukasPaczos for Maps 👀

danesfeder · 2018-10-03T19:35:48Z

@tobrun @LukasPaczos

Processing the traffic data has seemed to help to UI thread blockage by a decent amount, there is still some blockage but most likely by Part 2:

If either of you have a minute to check this code out, after my changes, I'm wondering if there's anything glaring causing this behavior I'm seeing:

It seems the previous route geometry is being drawn before the new updated geometry is drawn. I'm clearing all of the data for the feature collections, so I've been having a tough time tracking this one down. The NavigationMapRoute still needs some cleanup for #928, so hopefully the bug above will iron itself out with a bit more organization.

danesfeder · 2018-11-02T18:47:00Z

Update here - had an awesome pairing session with @LukasPaczos and we were able to identify some great areas of optimization.

It seems the previous route geometry is being drawn before the new updated geometry is drawn. I'm clearing all of the data for the feature collections, so I've been having a tough time tracking this one down.

Also were able to track this down ^ thanks for the eyes on that @LukasPaczos!

danesfeder · 2018-11-06T22:21:23Z

@LukasPaczos I made a lot of the updates here, but struggled with the Source / Layer setup we discussed. My main struggles were around handling the alternative route selection (differentiating between two alternative route layers in the same alternative Source). I opted to go with the previous setup, but refactored a bit.

I removed the Feature data that wasn't necessary, only including the data used by the expressions.

The last piece to this puzzle is the bug fix we discussed. The fix is we need to notify the GeoJsonSource that we removed the layers right? I hit a snag trying to figure this out. How do we "refresh" the source when there is no data to refresh it with? Thanks for the help, this is almost done!

LukasPaczos · 2018-11-07T17:38:35Z

My main struggles were around handling the alternative route selection (differentiating between two alternative route layers in the same alternative Source). I opted to go with the previous setup, but refactored a bit.

In order to achieve that using only one source, you'll need to add a property to each "CongestionFeature" that indicates, that this feature is a part of a selected, or an alternative route. Then, you need to verify that in your layer setup.

Feature feature = Feature.fromGeometry(congestionLineString);
...
feature.addBooleanProperty(PRIMARY_ROUTE_PROPERTY, isPrimary);

PropertyFactory.lineColor(
  switchCase(
    get(PRIMARY_ROUTE_PROPERTY), match(
      Expression.toString(get(RouteConstants.CONGESTION_KEY)),
      color(routeDefaultColor),
      stop("moderate", color(routeModerateColor)),
      stop("heavy", color(routeSevereColor)),
      stop("severe", color(routeSevereColor))
    ),
    match(
      Expression.toString(get(RouteConstants.CONGESTION_KEY)),
      color(alternativeRouteDefaultColor),
      stop("moderate", color(alternativeRouteModerateColor)),
      stop("heavy", color(alternativeRouteSevereColor)),
      stop("severe", color(alternativeRouteSevereColor))
    )
  )
)

Haven't tested it, but I believe setting the layer up this way, would let you add it to the map just once instead of re-adding it with each route selection, because you are only using data-driven values for the segments' colors. All you need to do, is reset the PRIMARY_ROUTE_PROPERTY property and update the source.

The fix is we need to notify the GeoJsonSource that we removed the layers right? I hit a snag trying to figure this out. How do we "refresh" the source when there is no data to refresh it with?

If coupled with the above setup, there would be no need for removing the layer! But besides that, have you tried reseting the source with an empty collection once clearRouteListData is called?

danesfeder · 2018-12-06T18:46:46Z

Yeah that seems to have fixed it, thanks @tobrun

danesfeder · 2018-12-06T19:00:28Z

This is ready for review!

LukasPaczos

Thanks for patience @danesfeder!

I was able to take this one for another spin - generally, setup looks good, but I found some issues. The major one being that the NavigationMapRouteActivity crashes on the device rotation.

Above is caused by the MapRouteLine leak - when the activity is recreated it draws an initial line and immediately schedules a redraw of all of the features because style loaded callback is delivered (the nature of this double invocation is a separate issue). This needs to go through the FeatureProcessingTask that keeps a reference to the MapRouteLine via the anonymous inner class callback. When this callback is fired, it's going to try and update sources that are invalid since a new instance of the MapRouteLine is created in the redraw method.
To avoid that, we need to store a weak reference to the listener instead. This shows, however, that a reference to the MapRouteLine is still kept in the MapRouteClickListener and MapRouteProgressChangeListener. Those 2 instances will have to be recreated and reassigned when redrawing after a new style has been loaded.

The same logic will have to be applied to the PrimaryRouteUpdateTask as well.

Another pain point I can see is an unlikely scenario, where the route is tried to be drawn after the style change has been started and before it's finished. We'll need to listen for the style load started callback and abort any source updates until it has finished loading.

Here's a diff of the changes needed to be done to prevent leaks of the FeatureProcessingTask.
This is just an example - initialization of the listeners in the NavigationMapRoute should probably be cleaned up.

LukasPaczos · 2018-12-11T13:00:04Z

...n-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/map/NavigationMapboxMap.java

@@ -421,6 +426,14 @@ public void onStop() {
    mapWayName.onStop();
  }

+  /**
+   * Should be used in {@link FragmentActivity#onDestroy()} to ensure proper


Javadoc should also mention that in case of Fragments, this should be called from the #onDestroyView.

LukasPaczos · 2018-12-11T13:20:17Z

...ation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/MapRouteArrow.java

+  private List<Layer> arrowLayers;
+  private GeoJsonSource arrowShaftGeoJsonSource;
+  private GeoJsonSource arrowHeadGeoJsonSource;
+  private Feature arrowShaftGeoJsonFeature = Feature.fromGeometry(Point.fromLngLat(0, 0));


arrowShaftGeoJsonFeature and arrowHeadGeoJsonFeature can be local variables and sources can be initialized with an empty feature instead of holding the reference to those features.

LukasPaczos · 2018-12-11T13:21:26Z

...ation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/MapRouteArrow.java

+    );
+  }
+
+  private void initializeArrowLayers(LineLayer shaftLayer, LineLayer shaftCasingLayer, SymbolLayer headLayer,


NIT: this method name is a bit misleading

LukasPaczos · 2018-12-11T13:42:28Z

.../src/main/java/com/mapbox/services/android/navigation/ui/v5/route/MapRouteClickListener.java

+    return routeLineStrings == null || routeLineStrings.isEmpty() || !alternativesVisible;
+  }
+
+  private void findClickedRoute(@NonNull LatLng point, HashMap<LineString, DirectionsRoute> routeLineStrings,


I'm not confident that this is the best way to determine which route was selected. With the "closest point to route" approach the route is going to be selected even if the users clicks far off the route, just because it is the closest one. This method also scales poorly when the routes get longer and longer because we are adding more points to the calculation.

I'd rather leverage map query and try using something like MapboxMap#queryRenderedFeatures with a similar filtering that we are using for drawing the routes. That said, I don't think this is a blocker for this PR because it's copied from the previous implementation, but definitely worth tackling in the future.

This sounds great - let's ticket this out, while keeping in mind using this implementation with a bounding box setup to increase the click region.

ticketed in #1634

LukasPaczos · 2018-12-11T13:46:33Z

...gation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/MapRouteLine.java

+    updateAllLayersVisibilityTo(isVisible);
+  }
+
+  boolean retrieveVisibilty() {


LukasPaczos · 2018-12-11T13:53:48Z

.../src/main/java/com/mapbox/services/android/navigation/ui/v5/route/FeatureProcessingTask.java

+    callback.onRouteFeaturesProcessed(routeFeatureCollections, routeLineStrings);
+  }
+
+  private FeatureCollection createRouteFeatureCollection(DirectionsRoute route, boolean isPrimary) {


Following the code path, it seems like this method can just return a Feature. MapRouteLine is generating a FeatureCollection regardless and it would allow us to drop this one.

@LukasPaczos maybe I'm not following completely, but I think this needs to be a collection? This task is generating a list of Features based on traffic congestion and then pushing them to MapRouteLine. I couldn't find the get(0) you spoke of during paring - sorry if I'm just not seeing it.

I think you are right, not sure what I've seen here before... 🤔

LukasPaczos · 2018-12-11T13:56:33Z

...gation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/MapRouteLine.java

+      for (int i = 0; i < styleLayers.size(); i++) {
+        if (!(styleLayers.get(i) instanceof SymbolLayer)
+          // Avoid placing the route on top of the user location layer
+          && !styleLayers.get(i).getId().contains(RouteConstants.MAPBOX_LOCATION_ID)) {


We should expose the LocationComponent layer constant!

danesfeder · 2018-12-14T22:44:20Z

@LukasPaczos this is up-to-date now with your changes, thanks again! My only remaining question is #1387 (comment)

devotaaabel · 2018-12-13T21:14:34Z

.../src/main/java/com/mapbox/services/android/navigation/ui/v5/route/FeatureProcessingTask.java

+      if (leg.annotation() != null && leg.annotation().congestion() != null) {
+        for (int i = 0; i < leg.annotation().congestion().size(); i++) {
+          // See https://github.com/mapbox/mapbox-navigation-android/issues/353
+          if (leg.annotation().congestion().size() + 1 <= lineString.coordinates().size()) {


What's wrong with if (leg.annotation().congestion().size() < lineString.coordinates().size()) { ?

devotaaabel · 2018-12-14T20:55:34Z

...-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/NavigationMapRoute.java

+   *
+   * @param isVisible true to show routes, false to hide
+   */
+  public void updateRouteVisibilityTo(boolean isVisible) {


wouldn't it be more consistent with other visibility modifiers for these to be named updateRouteVisibility or showRoute?

devotaaabel · 2018-12-14T20:59:56Z

...-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/NavigationMapRoute.java

+   *
+   * @param isVisible true to show routes, false to hide
+   */
+  public void updateRouteArrowVisibilityTo(boolean isVisible) {


devotaaabel · 2018-12-14T21:13:54Z

.../src/main/java/com/mapbox/services/android/navigation/ui/v5/route/MapRouteClickListener.java

+import java.util.HashMap;
+import java.util.List;
+
+class MapRouteClickListener implements MapboxMap.OnMapClickListener {


I know this implements OnMapClickListener but I'm not sure Listener is a good way to describe this class, maybe MapRouteClickHandler or something?

devotaaabel · 2018-12-14T21:19:10Z

...-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/NavigationMapRoute.java

+    this.routeArrow = new MapRouteArrow(mapView, mapboxMap, styleRes);
+    this.mapRouteClickListener = new MapRouteClickListener(routeLine);
+    this.mapRouteProgressChangeListener = new MapRouteProgressChangeListener(routeLine, routeArrow);
+    initializeDidFinishLoadingStyleListener();


maybe combine the three lines above and just have a method initializeListeners

devotaaabel · 2018-12-14T22:01:09Z

.../java/com/mapbox/services/android/navigation/ui/v5/route/MapRouteProgressChangeListener.java

@@ -10,11 +10,13 @@

 class MapRouteProgressChangeListener implements ProgressChangeListener {


Same comment for this class, I don't really think this qualifies as a Listener since it's not just listening, it's taking action

devotaaabel · 2018-12-14T23:44:21Z

.../src/main/java/com/mapbox/services/android/navigation/ui/v5/route/FeatureProcessingTask.java

+  @Override
+  protected void onPostExecute(Void result) {
+    super.onPostExecute(result);
+    Runtime.getRuntime().gc();


Why is it manually garbage collecting?

@devotaaabel this is needed to avoid leaks 👀 #1387 (review)

@danesfeder let's add a TODO comment here stating that would be 💯 to find another approach that doesn't need to call the garbage collector explicitly.

Guardiola31337

Overall this looks awesome 💯 Can't wait to merge this in! Good job @danesfeder 👏
Great refactoring extracting MapRouteArrow and MapRouteLine out from NavigationMapRoute ❤️ ❤️

I left some minor comments / questions that we can discuss.

Guardiola31337 · 2018-12-18T11:19:35Z

.../src/main/java/com/mapbox/services/android/navigation/ui/v5/route/FeatureProcessingTask.java

+  @Override
+  protected void onPostExecute(Void result) {
+    super.onPostExecute(result);
+    Runtime.getRuntime().gc();


@devotaaabel this is needed to avoid leaks 👀 #1387 (review)

@danesfeder let's add a TODO comment here stating that would be 💯 to find another approach that doesn't need to call the garbage collector explicitly.

Guardiola31337 · 2018-12-18T11:28:09Z

...gation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/MapRouteLine.java

+    this.routeLayers = new ArrayList<>();
+
+    TypedArray typedArray = context.obtainStyledAttributes(styleRes, R.styleable.NavigationMapRoute);
+    // Primary Route attributes


What about extracting this block of code into a private method and give it a name based on the comment? This way the constructor will be easier to read and understand and comment will become superfluous so it won't be necessary.

Guardiola31337 · 2018-12-18T11:29:01Z

...gation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/MapRouteLine.java

+      ContextCompat.getColor(context, R.color.mapbox_navigation_route_shield_layer_color));
+    routeScale = typedArray.getFloat(R.styleable.NavigationMapRoute_routeScale, 1.0f);
+
+    // Secondary Routes attributes


Same here re: extract method based on comment

Guardiola31337 · 2018-12-18T11:32:54Z

...gation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/MapRouteLine.java

+    alternativeRouteScale = typedArray.getFloat(
+      R.styleable.NavigationMapRoute_alternativeRouteScale, 1.0f);
+
+    // Waypoint attributes


Same here re: extract method based on comment

Guardiola31337 · 2018-12-18T11:35:29Z

...src/main/java/com/mapbox/services/android/navigation/ui/v5/route/PrimaryRouteUpdateTask.java

+  @Override
+  protected List<FeatureCollection> doInBackground(Void... voids) {
+    List<FeatureCollection> updatedRouteCollections = new ArrayList<>(routeFeatureCollections);
+    // Update the primary new collection


What about extracting this block of code into a private method and give it a name based on the comment? This way doInBackground will be easier to read and understand and comment will become superfluous so it won't be necessary.

Guardiola31337 · 2018-12-18T11:38:02Z

...src/main/java/com/mapbox/services/android/navigation/ui/v5/route/PrimaryRouteUpdateTask.java

+
+  @Override
+  protected void onPostExecute(List<FeatureCollection> updatedRouteCollections) {
+    Runtime.getRuntime().gc();


What about dropping a comment here (TODO?) explaining why we're calling the garbage collector explicitly?

Guardiola31337 · 2018-12-18T11:38:31Z

...src/main/java/com/mapbox/services/android/navigation/ui/v5/route/PrimaryRouteUpdateTask.java

+    for (Feature feature : primaryFeatures) {
+      feature.addBooleanProperty(PRIMARY_ROUTE_PROPERTY_KEY, true);
+    }
+    // Update non-primary collections (not including the primary)


Same here re: extract method based on comment

Guardiola31337 · 2018-12-18T11:49:35Z

...tion-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/RouteConstants.java

@@ -0,0 +1,49 @@
+package com.mapbox.services.android.navigation.ui.v5.route;
+
+class RouteConstants {


Comment - IMO constants, objects and methods should live / be where their context is. From my experience constants classes as well as utility classes become a drawer easily and we should treat them carefully.

Guardiola31337 · 2018-12-18T11:54:46Z

...on-ui/src/test/java/com/mapbox/services/android/navigation/ui/v5/route/MapRouteLineTest.java

+import static org.mockito.Mockito.when;
+
+@RunWith(RobolectricTestRunner.class)
+public class MapRouteLineTest extends BaseTest {


Could you extract (if possible) some of the shared arrange code across these tests into a private factory method? It seems there's some duplicated code used across almost all the tests 👇

Guardiola31337 · 2018-12-18T12:42:32Z

...-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/NavigationMapRoute.java

-      MapUtils.addLayerToMap(mapboxMap, waypointLayer, belowLayer);
-    }
+  @OnLifecycleEvent(Lifecycle.Event.ON_DESTROY)
+  public void onDestroy() {


I believe this was added to workaround 👇 commented in #1387 (comment)

I updated the MapRouteLine and MapRouteArrow to each have an onDestroy that aims to clean up the layers. I also updated the arrow code to remove the layers if we find they exist.

Question (especially for @LukasPaczos) - Shouldn't this be handled gracefully from the Maps SDK side when calling MapboxMap#onDestroy? Have we explored this option? I can cut a ticket in mapbox-gl-native to discuss further if you think this should be addressed upstream (if there isn't one already 😅). Does that make sense? Let me know what you think.

Correct, there is no need for clearing the layers on the actual MapView destroy. You can even see logs like

E/Mbgl-NativeMapView: You're calling `removeLayer` after the `MapView` was destroyed, were you invoking it after `onDestroy()`?

because it's called after the call to MapView#onDestroy and it doesn't have any real impact.

I believe that the initial idea here was to clear layers when the NavigationMapRoute is detached from the map so that the navigation SDK doesn't leave unused layers on the map that can still be in use otherwise. If a solution for this minor use-case seems not so relevant in the grand scheme of things, it can definitely be tackled by a separate PR.

Sorry if I didn't make myself clear. I was wondering if the Maps SDK should be in charge of the "layers cleanup" somehow - ensuring that any layers added are removed properly when in onDestroy. Probably it has more implications / considerations that I'm not taking into account / missing but IMO this should be maps responsibility - obviously calling removeLayer afterwards is a dev implementation mistake (even in this case which doesn't seem to affect - other than the logs printed out). Is that what you're saying? What am I missing? I'm confused 😅

I believe that the initial idea here was to clear layers when the NavigationMapRoute is detached from the map so that the navigation SDK doesn't leave unused layers on the map that can still be in use otherwise.

Not sure if I follow, I think ☝️ was the part that make me 😕 Could you clarify?

If a solution for this minor use-case seems not so relevant in the grand scheme of things, it can definitely be tackled by a separate PR.

Sure thing! This was a question 🤔 that I wanted to fully understand. Not a blocker at all, I agree.

I was wondering if the Maps SDK should be in charge of the "layers cleanup" somehow - ensuring that any layers added are removed properly when in onDestroy.

This is already the case. There is no need for a manual layer cleanup when the map is destroyed, that's what I meant above.

Not sure if I follow, I think ☝️ was the part that make me 😕 Could you clarify?

I was going with an impression that the NavigationMapRoute is a standalone object that can be added and removed from the map. If that's not the case, there's nothing to worry about :)

This is already the case. There is no need for a manual layer cleanup when the map is destroyed, that's what I meant above.

Nice! Thanks for confirming. In any case, I don't see where the Maps SDK does this cleanup. That's why I asked.

I was going with an impression that the NavigationMapRoute is a standalone object that can be added and removed from the map.

Nah, not really. What you can do is show / hide both the MapRouteLine and MapRouteArrow which translates internally to set the visibility of the layer accordingly.

If that's not the case, there's nothing to worry about :)

Do you mean that if that's the case we shouldn't worry about removing the layers and the sources explicitly when in onDestroy? This is what we're currently doing in

mapbox-navigation-android/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/NavigationMapRoute.java

Lines 315 to 323 in 2540c5a

/**

* This method should be added in your {@link Activity#onDestroy()} or

* {@link android.support.v4.app.Fragment#onDestroyView()} to handle removing resources that were added to the map.

*/

@OnLifecycleEvent(Lifecycle.Event.ON_DESTROY)

public void onDestroy() {

routeLine.onDestroy();

routeArrow.onDestroy();

}

Yes, if the map will never outlive MapRouteLine, we shouldn't worry about removing those layers.

Got it! Thanks @LukasPaczos 🙏

cc'ing @danesfeder to double check / confirm and remove.

LukasPaczos · 2018-12-18T14:42:58Z

Thanks for addressing previous comments!

I found another 2 minor issues:

The previous routes are being displayed after the style load (even though they were previously hidden).
Sometimes the alternative routes are not displayed and show up only after the style reload. This isn't reproducible on demand, but every 5 route generations or so the issues shows itself. Also, a minor issue, that a selected route is not saved across style changes.

danesfeder · 2018-12-20T01:50:07Z

@LukasPaczos great catches! I believe I was able to address both of them with this latest round of updates. What was happening is we were trying to recreate these options (visibility, primary route, etc.) before the feature processing task was finished.

Previous route not being displayed / primary route selection maintained

LukasPaczos · 2018-12-20T12:20:41Z

LGTM, thanks for the fantastic work @danesfeder!

Leaving to final review to the nav folks.

Guardiola31337

Overall this looks good to me. I've retested and it's working 💯

Would you mind @LukasPaczos to do a quick final check 👀 before we merge here? Thanks in advance 🙏

Although some feedback isn't tackled, let's revisit later / create a follow-up PR to address it. They're minor comments mainly around naming so shouldn't block the PR.

Thanks again @danesfeder for the great work 👏

Guardiola31337 · 2018-12-20T12:28:58Z

Would you mind @LukasPaczos to do a quick final check 👀 before we merge here? Thanks in advance 🙏

LGTM, thanks for the fantastic work @danesfeder!

You beat me @LukasPaczos 😅 Thanks for your help here, much appreciated!

As mentioned above, going ahead and merging 🚀

danesfeder added bug Defect to be fixed. ⚠️ DO NOT MERGE PR should not be merged! navigation-ui labels Oct 3, 2018

danesfeder self-assigned this Oct 3, 2018

danesfeder requested review from Guardiola31337 and devotaaabel October 3, 2018 17:43

danesfeder force-pushed the dan-map-route branch from 8455195 to a528c88 Compare October 3, 2018 19:25

danesfeder modified the milestones: 0.20.0, 0.21.0 Oct 4, 2018

danesfeder modified the milestones: 0.21.0, 0.22.0 Oct 16, 2018

danesfeder force-pushed the dan-map-route branch from a528c88 to 808119e Compare October 23, 2018 19:21

danesfeder modified the milestones: 0.22.0, v0.23.0 Oct 25, 2018

danesfeder force-pushed the dan-map-route branch 3 times, most recently from 3d24d10 to 29ca858 Compare October 30, 2018 14:25

danesfeder mentioned this pull request Oct 30, 2018

Question: NavigationMapRoute Line #1293

Closed

danesfeder force-pushed the dan-map-route branch from 29ca858 to 5459faa Compare November 6, 2018 22:17

danesfeder force-pushed the dan-map-route branch from 5459faa to 1dfe6e1 Compare November 6, 2018 22:34

danesfeder modified the milestones: v0.23.0, v0.24.0 Nov 7, 2018

danesfeder requested a review from LukasPaczos November 8, 2018 15:18

danesfeder force-pushed the dan-map-route branch from 1dfe6e1 to 90f28c5 Compare November 8, 2018 20:46

danesfeder mentioned this pull request Nov 9, 2018

Running real machine, The console prints the following information #1512

Closed

danesfeder modified the milestones: v0.24.0, v0.25.0 Dec 7, 2018

danesfeder force-pushed the dan-map-route branch from eed90a3 to 44db396 Compare December 10, 2018 23:49

LukasPaczos reviewed Dec 11, 2018

View reviewed changes

danesfeder force-pushed the dan-map-route branch from 44db396 to 318953c Compare December 14, 2018 22:32

danesfeder mentioned this pull request Dec 14, 2018

Use MapboxMap#queryRenderedFeatures for NavigationMapRoute click listener #1634

Closed

danesfeder force-pushed the dan-map-route branch from 318953c to 2848504 Compare December 14, 2018 22:53

danesfeder removed the backwards incompatible Requires a SEMVER major version change. label Dec 14, 2018

devotaaabel reviewed Dec 14, 2018

View reviewed changes

danesfeder force-pushed the dan-map-route branch from 2848504 to 2540c5a Compare December 17, 2018 18:35

Guardiola31337 reviewed Dec 18, 2018

View reviewed changes

danesfeder modified the milestones: v0.25.0, v0.26.0 Dec 18, 2018

danesfeder added 3 commits December 19, 2018 18:35

Refactor NavigationMapRoute

6e9b714

Add / update lifecycle methods for NavigationMapRoute

b2cacfb

Remove GeoJson sources onDestroy

b4d4db7

danesfeder force-pushed the dan-map-route branch from 2540c5a to b4d4db7 Compare December 20, 2018 01:47

Guardiola31337 approved these changes Dec 20, 2018

View reviewed changes

Guardiola31337 merged commit 764d1ba into master Dec 20, 2018

Guardiola31337 deleted the dan-map-route branch December 20, 2018 12:29

Guardiola31337 mentioned this pull request Dec 20, 2018

Release v0.26.0 #1644

Closed

12 tasks

danesfeder mentioned this pull request Jan 14, 2019

Keep sources in memory #928

Closed

Guardiola31337 mentioned this pull request May 29, 2019

Add check to avoid ArrayIndexOutOfBoundsExceptions from MapRouteLine#drawWayPoints #1951

Merged

10 tasks

		@@ -10,11 +10,13 @@

		class MapRouteProgressChangeListener implements ProgressChangeListener {

		@@ -0,0 +1,49 @@
		package com.mapbox.services.android.navigation.ui.v5.route;

		class RouteConstants {

	/**
	* This method should be added in your {@link Activity#onDestroy()} or
	* {@link android.support.v4.app.Fragment#onDestroyView()} to handle removing resources that were added to the map.
	*/
	@OnLifecycleEvent(Lifecycle.Event.ON_DESTROY)
	public void onDestroy() {
	routeLine.onDestroy();
	routeArrow.onDestroy();
	}

NavigationMapRoute 2.0 #1387

NavigationMapRoute 2.0 #1387

Conversation

danesfeder commented Oct 3, 2018 • edited Loading

danesfeder commented Oct 3, 2018

danesfeder commented Nov 2, 2018

danesfeder commented Nov 6, 2018

LukasPaczos commented Nov 7, 2018

danesfeder commented Dec 6, 2018

danesfeder commented Dec 6, 2018

LukasPaczos left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danesfeder commented Dec 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Guardiola31337 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Guardiola31337 Dec 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LukasPaczos commented Dec 18, 2018

danesfeder commented Dec 20, 2018

LukasPaczos commented Dec 20, 2018

Guardiola31337 left a comment

Choose a reason for hiding this comment

Guardiola31337 commented Dec 20, 2018

danesfeder commented Oct 3, 2018 •

edited

Loading

LukasPaczos left a comment •

edited

Loading

Guardiola31337 left a comment •

edited

Loading

Guardiola31337 Dec 18, 2018 •

edited

Loading