From 81f7f2d66c68c198515e64ca64412e429b04f4c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Paczos?= Date: Thu, 28 Feb 2019 16:45:38 +0100 Subject: wip --- .../main/java/com/mapbox/mapboxsdk/maps/Style.java | 19 ++++++------- .../com/mapbox/mapboxsdk/style/layers/Layer.java | 21 ++++---------- .../mapboxsdk/style/sources/GeoJsonSource.java | 12 -------- .../com/mapbox/mapboxsdk/style/sources/Source.java | 11 ++++---- .../mapbox/mapboxsdk/testapp/maps/StyleLoadTest.kt | 27 ++++++++++++++++-- platform/android/src/style/layers/layer.cpp | 33 +++++++++++++++++++++- platform/android/src/style/layers/layer.hpp | 7 +++++ .../android/src/style/sources/geojson_source.cpp | 3 ++ .../android/src/style/sources/image_source.cpp | 9 ++++++ platform/android/src/style/sources/source.cpp | 12 +++++++- platform/android/src/style/sources/source.hpp | 7 +++++ 11 files changed, 113 insertions(+), 48 deletions(-) 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 d6bb0a9f01..c58bab19f5 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 @@ -7,6 +7,7 @@ import android.support.annotation.IntRange; import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.support.annotation.StringDef; +import android.support.annotation.VisibleForTesting; import android.util.DisplayMetrics; import android.util.Pair; @@ -51,7 +52,8 @@ public class Style { * @param builder the builder used for creating this style * @param nativeMap the map object used to load this style */ - private Style(@NonNull Builder builder, @NonNull NativeMap nativeMap) { + @VisibleForTesting + Style(@NonNull Builder builder, @NonNull NativeMap nativeMap) { this.builder = builder; this.nativeMap = nativeMap; } @@ -442,18 +444,13 @@ public class Style { */ void onWillStartLoadingMap() { fullyLoaded = false; - for (Source source : sources.values()) { - if (source != null) { - source.setDetached(); - nativeMap.removeSource(source); - } + + for (Source source : nativeMap.getSources()) { + source.nativeSetDetached(); } - for (Layer layer : layers.values()) { - if (layer != null) { - layer.setDetached(); - nativeMap.removeLayer(layer); - } + for (Layer layer : nativeMap.getLayers()) { + layer.nativeSetDetached(); } for (Map.Entry bitmapEntry : images.entrySet()) { diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/layers/Layer.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/layers/Layer.java index d290c093a7..6ef331c131 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/layers/Layer.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/layers/Layer.java @@ -17,9 +17,6 @@ public abstract class Layer { @Keep private long nativePtr; - @Keep - private boolean invalidated; - private boolean detached; static { LibraryLoader.load(); @@ -43,10 +40,6 @@ public abstract class Layer { } public void setProperties(@NonNull PropertyValue... properties) { - if (detached) { - return; - } - checkThread(); if (properties.length == 0) { return; @@ -142,6 +135,12 @@ public abstract class Layer { @Keep protected native void nativeSetMaxZoom(float zoom); + @Keep + public native void nativeSetDetached(); + + @Keep + public native boolean nativeIsDetached(); + public long getNativePtr() { return nativePtr; } @@ -156,12 +155,4 @@ public abstract class Layer { return value; } } - - public void setDetached() { - detached = true; - } - - public boolean isDetached() { - return detached; - } } \ No newline at end of file diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/GeoJsonSource.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/GeoJsonSource.java index 9e809c2ce0..b7b979958d 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/GeoJsonSource.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/GeoJsonSource.java @@ -194,9 +194,6 @@ public class GeoJsonSource extends Source { * @param feature the GeoJSON {@link Feature} to set */ public void setGeoJson(Feature feature) { - if (detached) { - return; - } checkThread(); nativeSetFeature(feature); } @@ -208,9 +205,6 @@ public class GeoJsonSource extends Source { * @param geometry the GeoJSON {@link Geometry} to set */ public void setGeoJson(Geometry geometry) { - if (detached) { - return; - } checkThread(); nativeSetGeometry(geometry); } @@ -222,9 +216,6 @@ public class GeoJsonSource extends Source { * @param features the GeoJSON FeatureCollection */ public void setGeoJson(FeatureCollection features) { - if (detached) { - return; - } checkThread(); nativeSetFeatureCollection(features); } @@ -236,9 +227,6 @@ public class GeoJsonSource extends Source { * @param json the raw GeoJson FeatureCollection string */ public void setGeoJson(String json) { - if (detached) { - return; - } checkThread(); nativeSetGeoJsonString(json); } diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/Source.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/Source.java index 9667a33e09..ef2454d58e 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/Source.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/Source.java @@ -1,7 +1,6 @@ package com.mapbox.mapboxsdk.style.sources; import android.support.annotation.Keep; - import android.support.annotation.NonNull; import com.mapbox.mapboxsdk.LibraryLoader; @@ -15,8 +14,6 @@ public abstract class Source { @Keep private long nativePtr; - protected boolean detached; - static { LibraryLoader.load(); } @@ -85,7 +82,9 @@ public abstract class Source { @Keep protected native String nativeGetAttribution(); - public void setDetached() { - detached = true; - } + @Keep + public native void nativeSetDetached(); + + @Keep + public native boolean nativeIsDetached(); } diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/maps/StyleLoadTest.kt b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/maps/StyleLoadTest.kt index 84af279bd0..10be2f36ec 100644 --- a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/maps/StyleLoadTest.kt +++ b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/maps/StyleLoadTest.kt @@ -7,10 +7,11 @@ import com.mapbox.mapboxsdk.maps.MapboxMap import com.mapbox.mapboxsdk.maps.Style import com.mapbox.mapboxsdk.style.layers.SymbolLayer import com.mapbox.mapboxsdk.style.sources.GeoJsonSource -import com.mapbox.mapboxsdk.testapp.action.MapboxMapAction +import com.mapbox.mapboxsdk.testapp.action.MapboxMapAction.invoke import com.mapbox.mapboxsdk.testapp.activity.EspressoTest import com.mapbox.mapboxsdk.testapp.activity.espresso.EspressoTestActivity import com.mapbox.mapboxsdk.testapp.utils.TestingAsyncUtils +import junit.framework.Assert import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -29,7 +30,7 @@ class StyleLoadTest : EspressoTest() { @Test fun updateSourceAfterStyleLoad() { validateTestSetup() - MapboxMapAction.invoke(mapboxMap) { uiController: UiController, mapboxMap: MapboxMap -> + invoke(mapboxMap) { uiController: UiController, mapboxMap: MapboxMap -> val source = GeoJsonSource("id") val layer = SymbolLayer("id", "id") mapboxMap.setStyle(Style.Builder().withSource(source).withLayer(layer)) @@ -39,4 +40,26 @@ class StyleLoadTest : EspressoTest() { source.setGeoJson("{}") } } + + @Test + fun loadingNewStyle_sourcesDetached() { + invoke(mapboxMap) { _: UiController, mapboxMap: MapboxMap -> + val sources = mapboxMap.style!!.sources + mapboxMap.setStyle(Style.DARK) + for (source in sources) { + Assert.assertTrue(source.nativeIsDetached()) + } + } + } + + @Test + fun loadingNewStyle_layersDetached() { + invoke(mapboxMap) { _: UiController, mapboxMap: MapboxMap -> + val layers = mapboxMap.style!!.layers + mapboxMap.setStyle(Style.DARK) + for (layer in layers) { + Assert.assertTrue(layer.nativeIsDetached()) + } + } + } } \ No newline at end of file diff --git a/platform/android/src/style/layers/layer.cpp b/platform/android/src/style/layers/layer.cpp index 2f335b6ff3..ee8949328d 100644 --- a/platform/android/src/style/layers/layer.cpp +++ b/platform/android/src/style/layers/layer.cpp @@ -91,7 +91,19 @@ namespace android { return layer; } + void Layer::setDetached(jni::JNIEnv&) { + detached = true; + } + + jboolean Layer::isDetached(jni::JNIEnv&) { + return detached ? jni::jni_true : jni::jni_false; + } + void Layer::setLayoutProperty(jni::JNIEnv& env, const jni::String& jname, const jni::Object<>& jvalue) { + if (detached) { + return; + } + // Convert and set property optional error = layer.setLayoutProperty(jni::Make(env, jname), Value(env, jvalue)); if (error) { @@ -101,6 +113,10 @@ namespace android { } void Layer::setPaintProperty(jni::JNIEnv& env, const jni::String& jname, const jni::Object<>& jvalue) { + if (detached) { + return; + } + // Convert and set property optional error = layer.setPaintProperty(jni::Make(env, jname), Value(env, jvalue)); if (error) { @@ -110,6 +126,10 @@ namespace android { } void Layer::setFilter(jni::JNIEnv& env, const jni::Array>& jfilter) { + if (detached) { + return; + } + using namespace mbgl::style; using namespace mbgl::style::conversion; @@ -137,6 +157,9 @@ namespace android { } void Layer::setSourceLayer(jni::JNIEnv& env, const jni::String& sourceLayer) { + if (detached) { + return; + } layer.setSourceLayer(jni::Make(env, sourceLayer)); } @@ -157,10 +180,16 @@ namespace android { } void Layer::setMinZoom(jni::JNIEnv&, jni::jfloat zoom) { + if (detached) { + return; + } layer.setMinZoom(zoom); } void Layer::setMaxZoom(jni::JNIEnv&, jni::jfloat zoom) { + if (detached) { + return; + } layer.setMaxZoom(zoom); } @@ -189,7 +218,9 @@ namespace android { METHOD(&Layer::getMaxZoom, "nativeGetMaxZoom"), METHOD(&Layer::setMinZoom, "nativeSetMinZoom"), METHOD(&Layer::setMaxZoom, "nativeSetMaxZoom"), - METHOD(&Layer::getVisibility, "nativeGetVisibility") + METHOD(&Layer::getVisibility, "nativeGetVisibility"), + METHOD(&Layer::setDetached, "nativeSetDetached"), + METHOD(&Layer::isDetached, "nativeIsDetached") ); } diff --git a/platform/android/src/style/layers/layer.hpp b/platform/android/src/style/layers/layer.hpp index 0fb679152b..67baf789aa 100644 --- a/platform/android/src/style/layers/layer.hpp +++ b/platform/android/src/style/layers/layer.hpp @@ -35,6 +35,10 @@ public: style::Layer& get(); + void setDetached(jni::JNIEnv&); + + jboolean isDetached(jni::JNIEnv&); + void setLayoutProperty(jni::JNIEnv&, const jni::String&, const jni::Object<>& value); void setPaintProperty(jni::JNIEnv&, const jni::String&, const jni::Object<>& value); @@ -90,6 +94,9 @@ protected: // Map is set when the layer is retrieved or after adding to the map mbgl::Map* map; + + // new style has started loading, making this layer invalid + bool detached; }; /** diff --git a/platform/android/src/style/sources/geojson_source.cpp b/platform/android/src/style/sources/geojson_source.cpp index a9307afe67..ccd068da0c 100644 --- a/platform/android/src/style/sources/geojson_source.cpp +++ b/platform/android/src/style/sources/geojson_source.cpp @@ -177,6 +177,9 @@ namespace android { } void GeoJSONSource::setAsync(Update::Converter converterFn) { + if (detached) { + return; + } awaitingUpdate = std::make_unique( std::move(converterFn), std::make_unique>( diff --git a/platform/android/src/style/sources/image_source.cpp b/platform/android/src/style/sources/image_source.cpp index b42e0e5a51..7adc03d794 100644 --- a/platform/android/src/style/sources/image_source.cpp +++ b/platform/android/src/style/sources/image_source.cpp @@ -32,6 +32,9 @@ namespace android { ImageSource::~ImageSource() = default; void ImageSource::setURL(jni::JNIEnv& env, const jni::String& url) { + if (detached) { + return; + } // Update the core source source.as()->ImageSource::setURL(jni::Make(env, url)); } @@ -42,10 +45,16 @@ namespace android { } void ImageSource::setImage(jni::JNIEnv& env, const jni::Object& bitmap) { + if (detached) { + return; + } source.as()->setImage(Bitmap::GetImage(env, bitmap)); } void ImageSource::setCoordinates(jni::JNIEnv& env, const jni::Object& coordinatesObject) { + if (detached) { + return; + } source.as()->setCoordinates( LatLngQuad::getLatLngArray(env, coordinatesObject)); } diff --git a/platform/android/src/style/sources/source.cpp b/platform/android/src/style/sources/source.cpp index be4c0367fc..e9f0f4e4b8 100644 --- a/platform/android/src/style/sources/source.cpp +++ b/platform/android/src/style/sources/source.cpp @@ -142,6 +142,14 @@ namespace android { rendererFrontend = nullptr; } + void Source::setDetached(jni::JNIEnv&) { + detached = true; + } + + jboolean Source::isDetached(jni::JNIEnv&) { + return detached ? jni::jni_true : jni::jni_false; + } + void Source::registerNative(jni::JNIEnv& env) { // Lookup the class static auto& javaClass = jni::Class::Singleton(env); @@ -151,7 +159,9 @@ namespace android { // Register the peer jni::RegisterNativePeer(env, javaClass, "nativePtr", METHOD(&Source::getId, "nativeGetId"), - METHOD(&Source::getAttribution, "nativeGetAttribution") + METHOD(&Source::getAttribution, "nativeGetAttribution"), + METHOD(&Source::setDetached, "nativeSetDetached"), + METHOD(&Source::isDetached, "nativeIsDetached") ); // Register subclasses diff --git a/platform/android/src/style/sources/source.hpp b/platform/android/src/style/sources/source.hpp index 93b706425a..e93a1cd735 100644 --- a/platform/android/src/style/sources/source.hpp +++ b/platform/android/src/style/sources/source.hpp @@ -43,6 +43,10 @@ public: jni::Local getAttribution(jni::JNIEnv&); + void setDetached(jni::JNIEnv&); + + jboolean isDetached(jni::JNIEnv&); + protected: // Set on newly created sources until added to the map. std::unique_ptr ownedSource; @@ -55,6 +59,9 @@ protected: // RendererFrontend pointer is valid only when added to the map. AndroidRendererFrontend* rendererFrontend { nullptr }; + + // new style has started loading, making this source invalid + bool detached; }; } // namespace android -- cgit v1.2.1