[android] do not reinitialize camera/render mode if it hasn't changed

Camera/Render mode updates can be streamed from various different stateless sources and we should make sure to skip an update to a mode that's already set. This keeps the callbacks tidy and does not perform unnecessary work.
mapbox · Jul 31, 2019 · 4985127 · 4985127
1 parent f86f022
commit 4985127
Show file tree

Hide file tree

Showing 5 changed files with 107 additions and 41 deletions.
diff --git a/platform/android/CHANGELOG.md b/platform/android/CHANGELOG.md
@@ -6,6 +6,7 @@ Mapbox welcomes participation and contributions from everyone.  If you'd like to
 
 ### Bug fixes
  - Fixed flickering on style change for the same tile set [#15127](https://github.com/mapbox/mapbox-gl-native/pull/15127)
+ - Fix location render/camera modes being reinitialized even when the new mode is the same [#15266](https://github.com/mapbox/mapbox-gl-native/pull/15266)
 
 ## 8.1.1 - July 26, 2019
 [Changes](https://github.com/mapbox/mapbox-gl-native/compare/android-v8.1.0...android-v8.1.1) since [Mapbox Maps SDK for Android v8.1.0](https://github.com/mapbox/mapbox-gl-native/releases/tag/android-v8.1.0):

diff --git a/...boxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationCameraController.java b/...boxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationCameraController.java
@@ -102,6 +102,10 @@ void setCameraMode(@CameraMode.Mode final int cameraMode, @Nullable Location las
                      long transitionDuration,
                      @Nullable Double zoom, @Nullable Double bearing, @Nullable Double tilt,
                      @Nullable OnLocationCameraTransitionListener internalTransitionListener) {
+    if (this.cameraMode == cameraMode) {
+      return;
+    }
+
     final boolean wasTracking = isLocationTracking();
     this.cameraMode = cameraMode;
 

diff --git a/...pboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationLayerController.java b/...pboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationLayerController.java
@@ -111,11 +111,8 @@ void applyStyle(@NonNull LocationComponentOptions options) {
       removeLayers();
       addLayers();
       if (isHidden) {
-        for (String layerId : layerSet) {
-          setLayerVisibility(layerId, false);
-        }
+        hide();
       }
-      setRenderMode(renderMode);
     }
 
     this.options = options;
@@ -131,49 +128,24 @@ void applyStyle(@NonNull LocationComponentOptions options) {
     styleAccuracy(options.accuracyAlpha(), options.accuracyColor());
     styleScaling(options);
     determineIconsSource(options);
+
+    if (!isHidden) {
+      show();
+    }
   }
 
   void setRenderMode(@RenderMode.Mode int renderMode) {
-    int previousMode = this.renderMode;
+    if (this.renderMode == renderMode) {
+      return;
+    }
     this.renderMode = renderMode;
 
     if (!isHidden) {
-      boolean isStale = locationFeature.getBooleanProperty(PROPERTY_LOCATION_STALE);
-      switch (renderMode) {
-        case RenderMode.NORMAL:
-          styleForeground(options);
-          setLayerVisibility(SHADOW_LAYER, true);
-          setLayerVisibility(FOREGROUND_LAYER, true);
-          setLayerVisibility(BACKGROUND_LAYER, true);
-          setLayerVisibility(ACCURACY_LAYER, !isStale);
-          setLayerVisibility(BEARING_LAYER, false);
-          break;
-        case RenderMode.COMPASS:
-          styleForeground(options);
-          setLayerVisibility(SHADOW_LAYER, true);
-          setLayerVisibility(FOREGROUND_LAYER, true);
-          setLayerVisibility(BACKGROUND_LAYER, true);
-          setLayerVisibility(ACCURACY_LAYER, !isStale);
-          setLayerVisibility(BEARING_LAYER, true);
-          break;
-        case RenderMode.GPS:
-          styleForeground(options);
-          setLayerVisibility(SHADOW_LAYER, false);
-          setLayerVisibility(FOREGROUND_LAYER, true);
-          setLayerVisibility(BACKGROUND_LAYER, true);
-          setLayerVisibility(ACCURACY_LAYER, false);
-          setLayerVisibility(BEARING_LAYER, false);
-          break;
-        default:
-          break;
-      }
-
-      determineIconsSource(options);
-    }
-
-    if (previousMode != renderMode) {
-      internalRenderModeChangedListener.onRenderModeChanged(renderMode);
+      styleForeground(options);
+      show();
     }
+    determineIconsSource(options);
+    internalRenderModeChangedListener.onRenderModeChanged(renderMode);
   }
 
   int getRenderMode() {
@@ -186,7 +158,32 @@ int getRenderMode() {
 
   void show() {
     isHidden = false;
-    setRenderMode(renderMode);
+    boolean isStale = locationFeature.getBooleanProperty(PROPERTY_LOCATION_STALE);
+    switch (renderMode) {
+      case RenderMode.NORMAL:
+        setLayerVisibility(SHADOW_LAYER, true);
+        setLayerVisibility(FOREGROUND_LAYER, true);
+        setLayerVisibility(BACKGROUND_LAYER, true);
+        setLayerVisibility(ACCURACY_LAYER, !isStale);
+        setLayerVisibility(BEARING_LAYER, false);
+        break;
+      case RenderMode.COMPASS:
+        setLayerVisibility(SHADOW_LAYER, true);
+        setLayerVisibility(FOREGROUND_LAYER, true);
+        setLayerVisibility(BACKGROUND_LAYER, true);
+        setLayerVisibility(ACCURACY_LAYER, !isStale);
+        setLayerVisibility(BEARING_LAYER, true);
+        break;
+      case RenderMode.GPS:
+        setLayerVisibility(SHADOW_LAYER, false);
+        setLayerVisibility(FOREGROUND_LAYER, true);
+        setLayerVisibility(BACKGROUND_LAYER, true);
+        setLayerVisibility(ACCURACY_LAYER, false);
+        setLayerVisibility(BEARING_LAYER, false);
+        break;
+      default:
+        break;
+    }
   }
 
   void hide() {

diff --git a/...LAndroidSDK/src/test/java/com/mapbox/mapboxsdk/location/LocationCameraControllerTest.java b/...LAndroidSDK/src/test/java/com/mapbox/mapboxsdk/location/LocationCameraControllerTest.java
@@ -141,6 +141,32 @@ public void setCameraMode_internalCameraTrackingChangeListenerIsCalled() {
     verify(internalTrackingChangedListener).onCameraTrackingChanged(cameraMode);
   }
 
+  @Test
+  public void setCameraMode_doNotNotifyAboutDuplicates_NONE() {
+    OnCameraTrackingChangedListener internalTrackingChangedListener = mock(OnCameraTrackingChangedListener.class);
+    LocationCameraController camera = buildCamera(internalTrackingChangedListener);
+    camera.initializeOptions(mock(LocationComponentOptions.class));
+    int cameraMode = NONE;
+
+    camera.setCameraMode(cameraMode);
+    camera.setCameraMode(cameraMode);
+
+    verify(internalTrackingChangedListener, times(1)).onCameraTrackingChanged(cameraMode);
+  }
+
+  @Test
+  public void setCameraMode_doNotNotifyAboutDuplicates_TRACKING_GPS() {
+    OnCameraTrackingChangedListener internalTrackingChangedListener = mock(OnCameraTrackingChangedListener.class);
+    LocationCameraController camera = buildCamera(internalTrackingChangedListener);
+    camera.initializeOptions(mock(LocationComponentOptions.class));
+    int cameraMode = TRACKING_GPS;
+
+    camera.setCameraMode(cameraMode);
+    camera.setCameraMode(cameraMode);
+
+    verify(internalTrackingChangedListener, times(1)).onCameraTrackingChanged(cameraMode);
+  }
+
   @Test
   public void setCameraMode_cancelTransitionsWhenSet() {
     MapboxMap mapboxMap = mock(MapboxMap.class);

diff --git a/...GLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/location/LocationLayerControllerTest.java b/...GLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/location/LocationLayerControllerTest.java
@@ -594,6 +594,44 @@ public void onNewAccuracyRadiusValue_updateIgnoredWithInvalidRenderMode() {
       .addNumberProperty(PROPERTY_ACCURACY_RADIUS, accuracyRadiusValue);
   }
 
+  @Test
+  public void renderModeChanged_doNotNotifyAboutDuplicates_NORMAL() {
+    OnRenderModeChangedListener internalRenderModeChangedListener = mock(OnRenderModeChangedListener.class);
+    LayerSourceProvider sourceProvider = buildLayerProvider();
+    GeoJsonSource locationSource = mock(GeoJsonSource.class);
+    when(sourceProvider.generateSource(any(Feature.class))).thenReturn(locationSource);
+    LayerBitmapProvider bitmapProvider = mock(LayerBitmapProvider.class);
+    LocationComponentOptions options = mock(LocationComponentOptions.class);
+
+    LocationLayerController controller =
+      new LocationLayerController(mapboxMap, mapboxMap.getStyle(), sourceProvider, buildFeatureProvider(options),
+      bitmapProvider, options, internalRenderModeChangedListener);
+
+    controller.setRenderMode(RenderMode.NORMAL);
+    controller.setRenderMode(RenderMode.NORMAL);
+
+    verify(internalRenderModeChangedListener, times(1)).onRenderModeChanged(RenderMode.NORMAL);
+  }
+
+  @Test
+  public void renderModeChanged_doNotNotifyAboutDuplicates_GPS() {
+    OnRenderModeChangedListener internalRenderModeChangedListener = mock(OnRenderModeChangedListener.class);
+    LayerSourceProvider sourceProvider = buildLayerProvider();
+    GeoJsonSource locationSource = mock(GeoJsonSource.class);
+    when(sourceProvider.generateSource(any(Feature.class))).thenReturn(locationSource);
+    LayerBitmapProvider bitmapProvider = mock(LayerBitmapProvider.class);
+    LocationComponentOptions options = mock(LocationComponentOptions.class);
+
+    LocationLayerController controller =
+      new LocationLayerController(mapboxMap, mapboxMap.getStyle(), sourceProvider, buildFeatureProvider(options),
+      bitmapProvider, options, internalRenderModeChangedListener);
+
+    controller.setRenderMode(RenderMode.GPS);
+    controller.setRenderMode(RenderMode.GPS);
+
+    verify(internalRenderModeChangedListener, times(1)).onRenderModeChanged(RenderMode.GPS);
+  }
+
   private LayerFeatureProvider buildFeatureProvider(@NonNull LocationComponentOptions options) {
     LayerFeatureProvider provider = mock(LayerFeatureProvider.class);
     when(provider.generateLocationFeature(null, options)).thenReturn(mock(Feature.class));