From 49851274d2fdff549ee01b64c4f9ad70513cfe15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Paczos?= Date: Wed, 31 Jul 2019 12:01:59 +0300 Subject: [PATCH] [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. --- platform/android/CHANGELOG.md | 1 + .../location/LocationCameraController.java | 4 + .../location/LocationLayerController.java | 79 +++++++++---------- .../LocationCameraControllerTest.java | 26 ++++++ .../location/LocationLayerControllerTest.java | 38 +++++++++ 5 files changed, 107 insertions(+), 41 deletions(-) diff --git a/platform/android/CHANGELOG.md b/platform/android/CHANGELOG.md index beb08f0b686..da674b5aad0 100644 --- 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/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationCameraController.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationCameraController.java index d22f7a8514c..79d2e02bf11 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationCameraController.java +++ b/platform/android/MapboxGLAndroidSDK/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/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationLayerController.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationLayerController.java index c0c6017cd65..b18a7c47424 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationLayerController.java +++ b/platform/android/MapboxGLAndroidSDK/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/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/location/LocationCameraControllerTest.java b/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/location/LocationCameraControllerTest.java index 7fb018d235d..d1b8642c317 100644 --- a/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/location/LocationCameraControllerTest.java +++ b/platform/android/MapboxGLAndroidSDK/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/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/location/LocationLayerControllerTest.java b/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/location/LocationLayerControllerTest.java index aa0a07b73e8..0fbc47df555 100644 --- a/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/location/LocationLayerControllerTest.java +++ b/platform/android/MapboxGLAndroidSDK/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));