From 1b7b4d47dc724761725a9fbf6ef4b9a23294d502 Mon Sep 17 00:00:00 2001 From: Tobrun Date: Fri, 7 Dec 2018 19:25:29 +0200 Subject: [android] - optimise loaded style for location component, expose isFullyLoaded --- .../location/LocationCameraController.java | 1 - .../mapboxsdk/location/LocationComponent.java | 10 +- .../location/LocationLayerController.java | 7 +- .../java/com/mapbox/mapboxsdk/maps/MapboxMap.java | 4 +- .../main/java/com/mapbox/mapboxsdk/maps/Style.java | 133 +++++++++++++++++++-- .../mapboxsdk/location/LocationComponentTest.kt | 2 +- .../mapboxsdk/location/utils/MapboxTestingUtils.kt | 11 +- 7 files changed, 150 insertions(+), 18 deletions(-) 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 7a20ca8177..a50b510073 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 @@ -6,7 +6,6 @@ import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.support.annotation.VisibleForTesting; import android.view.MotionEvent; - import com.mapbox.android.gestures.AndroidGesturesManager; import com.mapbox.android.gestures.MoveGestureDetector; import com.mapbox.android.gestures.RotateGestureDetector; diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationComponent.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationComponent.java index 2391d6a86e..59ce2a900e 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationComponent.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationComponent.java @@ -854,7 +854,8 @@ public final class LocationComponent { */ public void onFinishLoadingStyle() { if (isComponentInitialized) { - locationLayerController.initializeComponents(options); + style = mapboxMap.getStyle(); + locationLayerController.initializeComponents(style, options); locationCameraController.initializeOptions(options); onLocationLayerStart(); } @@ -913,7 +914,10 @@ public final class LocationComponent { if (isComponentInitialized) { return; } - isComponentInitialized = true; + + if (!style.isFullyLoaded()) { + throw new IllegalStateException("Style hasn't fully loaded yet, can't initialize LocationComponent."); + } this.style = style; this.options = options; @@ -946,6 +950,8 @@ public final class LocationComponent { setRenderMode(RenderMode.NORMAL); setCameraMode(CameraMode.NONE); + isComponentInitialized = true; + onLocationLayerStart(); } 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 ed15e52a08..b67ce3da88 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 @@ -62,7 +62,7 @@ final class LocationLayerController implements MapboxAnimator.OnLayerAnimationsV private int renderMode; private final MapboxMap mapboxMap; - private final Style style; + private Style style; private final LayerSourceProvider layerSourceProvider; private final LayerBitmapProvider bitmapProvider; private LocationComponentOptions options; @@ -81,11 +81,12 @@ final class LocationLayerController implements MapboxAnimator.OnLayerAnimationsV this.layerSourceProvider = layerSourceProvider; this.bitmapProvider = bitmapProvider; this.locationFeature = featureProvider.generateLocationFeature(locationFeature, options); - initializeComponents(options); + initializeComponents(style, options); setRenderMode(RenderMode.NORMAL); } - void initializeComponents(LocationComponentOptions options) { + void initializeComponents(Style style, LocationComponentOptions options) { + this.style = style; addLocationSource(); addLayers(options.layerBelow()); applyStyle(options); diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java index 03fbdab3bd..d7a61af7eb 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java @@ -101,7 +101,7 @@ public final class MapboxMap { * Get the Style of the map asynchronously. */ public void getStyle(@NonNull Style.OnStyleLoaded onStyleLoaded) { - if (style != null && style.isStyleLoaded()) { + if (style != null && style.isFullyLoaded()) { onStyleLoaded.onStyleLoaded(style); } else { styleLoadedCallbacks.add(onStyleLoaded); @@ -118,7 +118,7 @@ public final class MapboxMap { */ @Nullable public Style getStyle() { - if (style == null || !style.isStyleLoaded()) { + if (style == null || !style.isFullyLoaded()) { return null; } else { return style; diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Style.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Style.java index 72de0b24c5..c803042242 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Style.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Style.java @@ -34,7 +34,7 @@ public class Style { private final HashMap layers = new HashMap<>(); private final HashMap images = new HashMap<>(); private final Builder builder; - private boolean styleLoaded; + private boolean fullyLoaded; /** * Private constructor to build a style object. @@ -54,6 +54,11 @@ public class Style { */ @NonNull public String getUrl() { + if (!fullyLoaded) { + // we are loading a new style + return ""; + } + return nativeMapView.getStyleUrl(); } @@ -64,6 +69,11 @@ public class Style { */ @NonNull public String getJson() { + if (!fullyLoaded) { + // we are loading a new style + return ""; + } + return nativeMapView.getStyleJson(); } @@ -78,6 +88,11 @@ public class Style { */ @NonNull public List getSources() { + if (!fullyLoaded) { + // we are loading a new style + return new ArrayList<>(); + } + return nativeMapView.getSources(); } @@ -87,6 +102,11 @@ public class Style { * @param source the source to add */ public void addSource(@NonNull Source source) { + if (!fullyLoaded) { + // we are loading a new style + return; + } + sources.put(source.getId(), source); nativeMapView.addSource(source); } @@ -99,6 +119,11 @@ public class Style { */ @Nullable public Source getSource(String id) { + if (!fullyLoaded) { + // we are loading a new style + return null; + } + Source source = sources.get(id); if (source == null) { source = nativeMapView.getSource(id); @@ -115,6 +140,11 @@ public class Style { */ @Nullable public T getSourceAs(@NonNull String sourceId) { + if (!fullyLoaded) { + // we are loading a new style + return null; + } + // noinspection unchecked if (sources.containsKey(sourceId)) { return (T) sources.get(sourceId); @@ -128,8 +158,12 @@ public class Style { * @param sourceId the source to remove * @return the source handle or null if the source was not present */ - @Nullable public boolean removeSource(@NonNull String sourceId) { + if (!fullyLoaded) { + // we are loading a new style + return false; + } + sources.remove(sourceId); return nativeMapView.removeSource(sourceId); } @@ -140,8 +174,12 @@ public class Style { * @param source the source to remove * @return the source */ - @Nullable public boolean removeSource(@NonNull Source source) { + if (!fullyLoaded) { + // we are loading a new style + return false; + } + sources.remove(source.getId()); return nativeMapView.removeSource(source); } @@ -156,6 +194,11 @@ public class Style { * @param layer the layer to add */ public void addLayer(@NonNull Layer layer) { + if (!fullyLoaded) { + // we are loading a new style + return; + } + layers.put(layer.getId(), layer); nativeMapView.addLayer(layer); } @@ -167,6 +210,11 @@ public class Style { * @param below the layer id to add this layer before */ public void addLayerBelow(@NonNull Layer layer, @NonNull String below) { + if (!fullyLoaded) { + // we are loading a new style + return; + } + layers.put(layer.getId(), layer); nativeMapView.addLayerBelow(layer, below); } @@ -178,6 +226,11 @@ public class Style { * @param above the layer id to add this layer above */ public void addLayerAbove(@NonNull Layer layer, @NonNull String above) { + if (!fullyLoaded) { + // we are loading a new style + return; + } + layers.put(layer.getId(), layer); nativeMapView.addLayerAbove(layer, above); } @@ -190,6 +243,11 @@ public class Style { * @param index the index to insert the layer at */ public void addLayerAt(@NonNull Layer layer, @IntRange(from = 0) int index) { + if (!fullyLoaded) { + // we are loading a new style + return; + } + layers.put(layer.getId(), layer); nativeMapView.addLayerAt(layer, index); } @@ -202,6 +260,11 @@ public class Style { */ @Nullable public Layer getLayer(@NonNull String id) { + if (!fullyLoaded) { + // we are loading a new style + return null; + } + Layer layer = layers.get(id); if (layer == null) { layer = nativeMapView.getLayer(id); @@ -218,6 +281,11 @@ public class Style { */ @Nullable public T getLayerAs(@NonNull String layerId) { + if (!fullyLoaded) { + // we are loading a new style + return null; + } + // noinspection unchecked return (T) nativeMapView.getLayer(layerId); } @@ -229,6 +297,11 @@ public class Style { */ @NonNull public List getLayers() { + if (!fullyLoaded) { + // we are loading a new style + return new ArrayList<>(); + } + return nativeMapView.getLayers(); } @@ -239,6 +312,10 @@ public class Style { * @return the removed layer or null if not found */ public boolean removeLayer(@NonNull String layerId) { + if (!fullyLoaded) { + // we are loading a new style + return false; + } layers.remove(layerId); return nativeMapView.removeLayer(layerId); } @@ -250,6 +327,10 @@ public class Style { * @return the layer */ public boolean removeLayer(@NonNull Layer layer) { + if (!fullyLoaded) { + // we are loading a new style + return false; + } layers.remove(layer.getId()); return nativeMapView.removeLayer(layer); } @@ -286,6 +367,11 @@ public class Style { * @param sdf the flag indicating image is an SDF or template image */ public void addImage(@NonNull String name, @NonNull Bitmap image, boolean sdf) { + if (!fullyLoaded) { + // we are loading a new style + return; + } + nativeMapView.addImage(name, image, sdf); } @@ -293,6 +379,11 @@ public class Style { * Adds an images to be used in the map's style. */ public void addImages(@NonNull HashMap images) { + if (!fullyLoaded) { + // we are loading a new style + return; + } + nativeMapView.addImages(images); } @@ -302,6 +393,11 @@ public class Style { * @param name the name of the image to remove */ public void removeImage(@NonNull String name) { + if (!fullyLoaded) { + // we are loading a new style + return; + } + nativeMapView.removeImage(name); } @@ -313,6 +409,11 @@ public class Style { */ @Nullable public Bitmap getImage(@NonNull String id) { + if (!fullyLoaded) { + // we are loading a new style + return null; + } + return nativeMapView.getImage(id); } @@ -329,6 +430,11 @@ public class Style { * @param transitionOptions the transition options */ public void setTransition(@NonNull TransitionOptions transitionOptions) { + if (!fullyLoaded) { + // we are loading a new style + return; + } + nativeMapView.setTransitionDuration(transitionOptions.getDuration()); nativeMapView.setTransitionDelay(transitionOptions.getDelay()); } @@ -343,6 +449,11 @@ public class Style { */ @NonNull public TransitionOptions getTransition() { + if (!fullyLoaded) { + // we are loading a new style + return new TransitionOptions(0, 0); + } + return new TransitionOptions(nativeMapView.getTransitionDuration(), nativeMapView.getTransitionDelay()); } @@ -357,6 +468,11 @@ public class Style { */ @Nullable public Light getLight() { + if (!fullyLoaded) { + // we are loading a new style + return null; + } + return nativeMapView.getLight(); } @@ -365,7 +481,6 @@ public class Style { * by setting the java sources and layers in a detached state and removing them from core. */ void onWillStartLoadingMap() { - styleLoaded = false; for (Source source : sources.values()) { if (source != null) { source.setDetached(); @@ -388,6 +503,8 @@ public class Style { sources.clear(); layers.clear(); images.clear(); + + fullyLoaded = false; } /** @@ -395,8 +512,8 @@ public class Style { * This method will add all components added to the builder that were defined with the 'with' prefix. */ void onDidFinishLoadingStyle() { - if (!styleLoaded) { - styleLoaded = true; + if (!fullyLoaded) { + fullyLoaded = true; for (Source source : builder.sources) { addSource(source); } @@ -424,8 +541,8 @@ public class Style { } } - boolean isStyleLoaded() { - return styleLoaded; + public boolean isFullyLoaded() { + return fullyLoaded; } // diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/location/LocationComponentTest.kt b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/location/LocationComponentTest.kt index 8362095042..94e1f079f3 100644 --- a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/location/LocationComponentTest.kt +++ b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/location/LocationComponentTest.kt @@ -153,7 +153,7 @@ class LocationComponentTest : BaseActivityTest() { executeComponentTest(componentAction) } - @Test + @Test(expected = IllegalStateException::class) fun settingMapStyleImmediatelyBeforeLoadingComponent_doesStillLoadLayersProperly() { validateTestSetup() val componentAction = object : LocationComponentAction.OnPerformLocationComponentAction { diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/location/utils/MapboxTestingUtils.kt b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/location/utils/MapboxTestingUtils.kt index 0a9aa48503..20ef5efded 100644 --- a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/location/utils/MapboxTestingUtils.kt +++ b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/location/utils/MapboxTestingUtils.kt @@ -48,6 +48,15 @@ fun MapboxMap.waitForLayer(uiController: UiController, location: Location, layer } } +fun MapboxMap.waitForStyle(uiController: UiController, mapboxMap: MapboxMap) { + var counter = 0 + val delay = MapboxTestingUtils.MAP_RENDER_DELAY + while ((mapboxMap.style == null && !mapboxMap.style?.isFullyLoaded!!) && delay * counter < MapboxTestingUtils.RENDER_TIMEOUT) { + uiController.loopMainThreadForAtLeast(delay) + counter++ + } +} + inline fun waitForRenderResult(uiController: UiController, checkFunction: () -> Boolean, expectedResult: Boolean) { var counter = 0 val delay = MapboxTestingUtils.MAP_RENDER_DELAY @@ -62,7 +71,7 @@ class MapboxTestingUtils { const val MAP_RENDER_DELAY = 250L const val MAP_CONNECTION_DELAY = 1000L - const val RENDER_TIMEOUT = 2_500L + const val RENDER_TIMEOUT = 5_500L /** * Used to increase style load time for stress testing. -- cgit v1.2.1