Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Commit

Permalink
nativeRemoveSource() and nativeRemoveLayer() methods should return a …
Browse files Browse the repository at this point in the history 
…boolean (#13428)
  • Loading branch information
@osana @tobrun
osana authored and tobrun committed Dec 10, 2018
1 parent 558ee1b commit aa9a647
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 84 deletions.
29 changes: 12 additions & 17 deletions platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,35 +351,32 @@ public void addLayerAt(@NonNull Layer layer, @IntRange(from = 0) int index) {
}

/**
* Removes the layer. Any references to the layer become invalid and should not be used anymore
* Removes the layer. The reference is re-usable after this and can be re-added
*
* @param layerId the layer to remove
* @return the removed layer or null if not found
* @return true if the layer was successfully removed, false - otherwise
*/
@Nullable
public Layer removeLayer(@NonNull String layerId) {
public boolean removeLayer(@NonNull String layerId) {
return nativeMapView.removeLayer(layerId);
}

/**
* Removes the layer. The reference is re-usable after this and can be re-added
*
* @param layer the layer to remove
* @return the layer
* @return true if the layer was successfully removed, false - otherwise
*/
@Nullable
public Layer removeLayer(@NonNull Layer layer) {
public boolean removeLayer(@NonNull Layer layer) {
return nativeMapView.removeLayer(layer);
}

/**
* Removes the layer. Any other references to the layer become invalid and should not be used anymore
*
* @param index the layer index
* @return the removed layer or null if not found
* @return true if the layer was successfully removed, false - otherwise
*/
@Nullable
public Layer removeLayerAt(@IntRange(from = 0) int index) {
public boolean removeLayerAt(@IntRange(from = 0) int index) {
return nativeMapView.removeLayerAt(index);
}

Expand Down Expand Up @@ -434,24 +431,22 @@ public void addSource(@NonNull Source source) {
}

/**
* Removes the source. Any references to the source become invalid and should not be used anymore
* Removes the source, preserving the reference for re-use
*
* @param sourceId the source to remove
* @return the source handle or null if the source was not present
* @return true if the source was successfully removed, false - otherwise
*/
@Nullable
public Source removeSource(@NonNull String sourceId) {
public boolean removeSource(@NonNull String sourceId) {
return nativeMapView.removeSource(sourceId);
}

/**
* Removes the source, preserving the reference for re-use
*
* @param source the source to remove
* @return the source
* @return true if the source was successfully removed, false - otherwise
*/
@Nullable
public Source removeSource(@NonNull Source source) {
public boolean removeSource(@NonNull Source source) {
return nativeMapView.removeSource(source);
}

Expand Down
52 changes: 23 additions & 29 deletions ...orm/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -748,27 +748,29 @@ public void addLayerAt(@NonNull Layer layer, @IntRange(from = 0) int index) {
nativeAddLayerAt(layer.getNativePtr(), index);
}

@Nullable
public Layer removeLayer(@NonNull String layerId) {
public boolean removeLayer(@NonNull String layerId) {
if (checkState("removeLayer")) {
return null;
return false;
}
return nativeRemoveLayerById(layerId);

Layer layer = getLayer(layerId);
if (layer != null) {
return removeLayer(layer);
}
return false;
}

@Nullable
public Layer removeLayer(@NonNull Layer layer) {

public boolean removeLayer(@NonNull Layer layer) {
if (checkState("removeLayer")) {
return null;
return false;
}
nativeRemoveLayer(layer.getNativePtr());
return layer;
return nativeRemoveLayer(layer.getNativePtr());
}

@Nullable
public Layer removeLayerAt(@IntRange(from = 0) int index) {
public boolean removeLayerAt(@IntRange(from = 0) int index) {
if (checkState("removeLayerAt")) {
return null;
return false;
}
return nativeRemoveLayerAt(index);
}
Expand All @@ -794,25 +796,22 @@ public void addSource(@NonNull Source source) {
nativeAddSource(source, source.getNativePtr());
}

@Nullable
public Source removeSource(@NonNull String sourceId) {
public boolean removeSource(@NonNull String sourceId) {
if (checkState("removeSource")) {
return null;
return false;
}
Source source = getSource(sourceId);
if (source != null) {
return removeSource(source);
}
return null;
return false;
}

@Nullable
public Source removeSource(@NonNull Source source) {
public boolean removeSource(@NonNull Source source) {
if (checkState("removeSource")) {
return null;
return false;
}
nativeRemoveSource(source, source.getNativePtr());
return source;
return nativeRemoveSource(source, source.getNativePtr());
}

public void addImage(@NonNull String name, @NonNull Bitmap image, boolean sdf) {
Expand Down Expand Up @@ -1217,16 +1216,11 @@ private native void nativeFlyTo(double angle, double latitude, double longitude,
@Keep
private native void nativeAddLayerAt(long layerPtr, int index) throws CannotAddLayerException;

@NonNull
@Keep
private native Layer nativeRemoveLayerById(String layerId);
private native boolean nativeRemoveLayer(long layerId);

@Keep
private native void nativeRemoveLayer(long layerId);

@NonNull
@Keep
private native Layer nativeRemoveLayerAt(int index);
private native boolean nativeRemoveLayerAt(int index);

@NonNull
@Keep
Expand All @@ -1240,7 +1234,7 @@ private native void nativeFlyTo(double angle, double latitude, double longitude,
private native void nativeAddSource(Source source, long sourcePtr) throws CannotAddSourceException;

@Keep
private native void nativeRemoveSource(Source source, long sourcePtr);
private native boolean nativeRemoveSource(Source source, long sourcePtr);

@Keep
private native void nativeAddImage(String name, Bitmap bitmap, float pixelRatio, boolean sdf);
Expand Down
37 changes: 25 additions & 12 deletions ...SDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/style/RuntimeStyleTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import static android.support.test.espresso.matcher.ViewMatchers.withId;
import static com.mapbox.mapboxsdk.testapp.action.MapboxMapAction.invoke;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
Expand Down Expand Up @@ -124,14 +125,13 @@ public void testRemoveLayerAt() {
public void perform(UiController uiController, View view) {
// Remove by index
Layer firstLayer = mapboxMap.getLayers().get(0);
Layer removed = mapboxMap.removeLayerAt(0);
assertNotNull(removed);
assertNotNull(removed.getId());
assertEquals(firstLayer.getId(), removed.getId());
boolean removed = mapboxMap.removeLayerAt(0);
assertTrue(removed);
assertNotNull(firstLayer);

// Test remove by index bounds checks
Timber.i("Remove layer at index > size");
assertNull(mapboxMap.removeLayerAt(Integer.MAX_VALUE));
assertFalse(mapboxMap.removeLayerAt(Integer.MAX_VALUE));
}
});
}
Expand Down Expand Up @@ -198,8 +198,8 @@ public void testAddRemoveSource() {
mapboxMap.addSource(new VectorSource("my-source", "mapbox://mapbox.mapbox-terrain-v2"));

// Remove
Source mySource = mapboxMap.removeSource("my-source");
assertNotNull(mySource);
boolean removeOk = mapboxMap.removeSource("my-source");
assertTrue(removeOk);
assertNull(mapboxMap.getLayer("my-source"));

// Add
Expand Down Expand Up @@ -288,9 +288,22 @@ public void testRemoveNonExistingSource() {
@Test
public void testRemoveNonExistingLayer() {
invoke(mapboxMap, (uiController, mapboxMap) -> {
mapboxMap.removeLayer("layer");
mapboxMap.removeLayerAt(mapboxMap.getLayers().size() + 1);
mapboxMap.removeLayerAt(-1);
assertFalse(mapboxMap.removeLayer("layer"));
assertFalse(mapboxMap.removeLayerAt(mapboxMap.getLayers().size() + 1));
assertFalse(mapboxMap.removeLayerAt(-1));
});
}

@Test
public void testRemoveExistingLayer() {
invoke(mapboxMap, (uiController, mapboxMap) -> {
Layer firstLayer = mapboxMap.getLayers().get(0);
assertTrue(mapboxMap.removeLayer(firstLayer));

firstLayer = mapboxMap.getLayers().get(0);
assertTrue(mapboxMap.removeLayer(firstLayer.getId()));

assertTrue(mapboxMap.removeLayerAt(0));
});
}

Expand Down Expand Up @@ -322,8 +335,8 @@ public void perform(UiController uiController, View view) {
assertNotNull(mapboxMap.getLayer("building"));

// Remove
Layer building = mapboxMap.removeLayer("building");
assertNotNull(building);
boolean removed = mapboxMap.removeLayer("building");
assertTrue(removed);
assertNull(mapboxMap.getLayer("building"));

// Add
Expand Down
32 changes: 13 additions & 19 deletions platform/android/src/native_map_view.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -824,50 +824,42 @@ void NativeMapView::addLayerAt(JNIEnv& env, jlong nativeLayerPtr, jni::jint inde
}
}

/**
* Remove by layer id.
*/
jni::Local<jni::Object<Layer>> NativeMapView::removeLayerById(JNIEnv& env, const jni::String& id) {
std::unique_ptr<mbgl::style::Layer> coreLayer = map->getStyle().removeLayer(jni::Make<std::string>(env, id));
if (coreLayer) {
return LayerManagerAndroid::get()->createJavaLayerPeer(env, *map, std::move(coreLayer));
} else {
return jni::Local<jni::Object<Layer>>();
}
}

/**
* Remove layer at index.
*/
jni::Local<jni::Object<Layer>> NativeMapView::removeLayerAt(JNIEnv& env, jni::jint index) {
jni::jboolean NativeMapView::removeLayerAt(JNIEnv& env, jni::jint index) {
auto layers = map->getStyle().getLayers();

// Check index
int numLayers = layers.size() - 1;
if (index > numLayers || index < 0) {
Log::Warning(Event::JNI, "Index out of range: %i", index);
return jni::Local<jni::Object<Layer>>();
return jni::jni_false;
}

std::unique_ptr<mbgl::style::Layer> coreLayer = map->getStyle().removeLayer(layers.at(index)->getID());
if (coreLayer) {
return LayerManagerAndroid::get()->createJavaLayerPeer(env, *map, std::move(coreLayer));
} else {
return jni::Local<jni::Object<Layer>>();
jni::Local<jni::Object<Layer>> layerObj =
LayerManagerAndroid::get()->createJavaLayerPeer(env, *map, std::move(coreLayer));
return jni::jni_true;
}
return jni::jni_false;
}

/**
* Remove with wrapper object id. Ownership is transferred back to the wrapper
*/
void NativeMapView::removeLayer(JNIEnv&, jlong layerPtr) {
jni::jboolean NativeMapView::removeLayer(JNIEnv&, jlong layerPtr) {
assert(layerPtr != 0);

mbgl::android::Layer *layer = reinterpret_cast<mbgl::android::Layer *>(layerPtr);
std::unique_ptr<mbgl::style::Layer> coreLayer = map->getStyle().removeLayer(layer->get().getID());
if (coreLayer) {
layer->setLayer(std::move(coreLayer));
return jni::jni_true;
}
return jni::jni_false;
}

jni::Local<jni::Array<jni::Object<Source>>> NativeMapView::getSources(JNIEnv& env) {
Expand Down Expand Up @@ -908,13 +900,16 @@ void NativeMapView::addSource(JNIEnv& env, const jni::Object<Source>& obj, jlong
}
}

void NativeMapView::removeSource(JNIEnv& env, const jni::Object<Source>& obj, jlong sourcePtr) {
jni::jboolean NativeMapView::removeSource(JNIEnv& env, const jni::Object<Source>& obj, jlong sourcePtr) {
assert(sourcePtr != 0);

mbgl::android::Source *source = reinterpret_cast<mbgl::android::Source *>(sourcePtr);
if (source->removeFromMap(env, obj, *map)) {
source->releaseJavaPeer();
return jni::jni_true;
}

return jni::jni_false;
}

void NativeMapView::addImage(JNIEnv& env, const jni::String& name, const jni::Object<Bitmap>& bitmap, jni::jfloat scale, jni::jboolean sdf) {
Expand Down Expand Up @@ -1046,7 +1041,6 @@ void NativeMapView::registerNative(jni::JNIEnv& env) {
METHOD(&NativeMapView::addLayer, "nativeAddLayer"),
METHOD(&NativeMapView::addLayerAbove, "nativeAddLayerAbove"),
METHOD(&NativeMapView::addLayerAt, "nativeAddLayerAt"),
METHOD(&NativeMapView::removeLayerById, "nativeRemoveLayerById"),
METHOD(&NativeMapView::removeLayerAt, "nativeRemoveLayerAt"),
METHOD(&NativeMapView::removeLayer, "nativeRemoveLayer"),
METHOD(&NativeMapView::getSources, "nativeGetSources"),
Expand Down
10 changes: 3 additions & 7 deletions platform/android/src/native_map_view.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,21 +216,17 @@ class NativeMapView : public MapObserver {

void addLayerAt(JNIEnv&, jni::jlong, jni::jint);

jni::Local<jni::Object<Layer>> removeLayerById(JNIEnv&, const jni::String&);
jni::jboolean removeLayerAt(JNIEnv&, jni::jint);

jni::Local<jni::Object<Layer>> removeLayerAt(JNIEnv&, jni::jint);

void removeLayer(JNIEnv&, jlong);
jni::jboolean removeLayer(JNIEnv&, jlong);

jni::Local<jni::Array<jni::Object<Source>>> getSources(JNIEnv&);

jni::Local<jni::Object<Source>> getSource(JNIEnv&, const jni::String&);

void addSource(JNIEnv&, const jni::Object<Source>&, jlong nativePtr);

jni::Local<jni::Object<Source>> removeSourceById(JNIEnv&, const jni::String&);

void removeSource(JNIEnv&, const jni::Object<Source>&, jlong nativePtr);
jni::jboolean removeSource(JNIEnv&, const jni::Object<Source>&, jlong nativePtr);

void addImage(JNIEnv&, const jni::String&, const jni::Object<Bitmap>& bitmap, jni::jfloat, jni::jboolean);

Expand Down

0 comments on commit aa9a647

Please sign in to comment.