[android] Update marker position and icon #3885

5 files changed, 153 insertions, 21 deletions

diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/annotations/Marker.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/annotations/Marker.java
index 27c9c03697..2cb9ff0ef6 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/annotations/Marker.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/annotations/Marker.java
@@ -61,8 +61,17 @@ public final class Marker extends Annotation {
         return infoWindowShown;
     }
 
-    void setPosition(LatLng position) {
+    /**
+     * Sets the position.
+     *
+     * @param position new position
+     */
+    public void setPosition(LatLng position) {
         this.position = position;
+        MapboxMap map = getMapboxMap();
+        if (map != null) {
+            map.updateMarker(this);
+        }
     }
 
     void setSnippet(String snippet) {
@@ -70,10 +79,16 @@ public final class Marker extends Annotation {
     }
 
     /**
-     * Do not use this method. Used internally by the SDK.
+     * Sets the icon.
+     *
+     * @param icon The icon to be used as Marker image
      */
     public void setIcon(@Nullable Icon icon) {
         this.icon = icon;
+        MapboxMap map = getMapboxMap();
+        if (map != null) {
+            map.updateMarker(this);
+        }
     }
 
     public Icon getIcon() {
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
index a7e75f5c8f..36478dce3e 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
@@ -31,6 +31,7 @@ import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.support.annotation.RequiresPermission;
 import android.support.annotation.UiThread;
+import android.support.v4.util.LongSparseArray;
 import android.support.v4.view.GestureDetectorCompat;
 import android.support.v4.view.ScaleGestureDetectorCompat;
 import android.support.v7.app.AlertDialog;
@@ -90,6 +91,10 @@ import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
 
 /**
  * <p>
@@ -116,7 +121,7 @@ public class MapView extends FrameLayout {
     private static final float DIMENSION_SEVENTYSIX_DP = 76f;
 
     private MapboxMap mMapboxMap;
-    private List<Annotation> mAnnotations;
+    private LongSparseArray<Annotation> mAnnotations;
     private List<Icon> mIcons;
 
     private NativeMapView mNativeMapView;
@@ -167,7 +172,7 @@ public class MapView extends FrameLayout {
     private void initialize(@NonNull Context context, @Nullable AttributeSet attrs) {
         mOnMapChangedListener = new CopyOnWriteArrayList<>();
         mMapboxMap = new MapboxMap(this);
-        mAnnotations = new ArrayList<>();
+        mAnnotations = new LongSparseArray<>();
         mIcons = new ArrayList<>();
 
         View view = LayoutInflater.from(context).inflate(R.layout.mapview_internal, this);
@@ -1007,6 +1012,11 @@ public class MapView extends FrameLayout {
 
     private Marker prepareMarker(MarkerOptions markerOptions) {
         Marker marker = markerOptions.getMarker();
+        ensureIconLoaded(marker);
+        return marker;
+    }
+
+    private void ensureIconLoaded(Marker marker) {
         Icon icon = marker.getIcon();
         if (icon == null) {
             icon = IconFactory.getInstance(getContext()).defaultMarker();
@@ -1021,8 +1031,12 @@ public class MapView extends FrameLayout {
                 throw new IconBitmapChangedException();
             }
         }
-        marker.setTopOffsetPixels(getTopOffsetPixelsForIcon(icon));
-        return marker;
+
+        // this seems to be a costly operation according to the profiler so I'm trying to save some calls
+        Marker previousMarker = marker.getId() != -1 ? (Marker) mAnnotations.get(marker.getId()) : null;
+        if (previousMarker == null || previousMarker.getIcon() == null || previousMarker.getIcon() != marker.getIcon()) {
+            marker.setTopOffsetPixels(getTopOffsetPixelsForIcon(icon));
+        }
     }
 
     /**
@@ -1047,12 +1061,40 @@ public class MapView extends FrameLayout {
         long id = mNativeMapView.addMarker(marker);
         marker.setId(id);        // the annotation needs to know its id
         marker.setMapboxMap(mMapboxMap); // the annotation needs to know which map view it is in
-        mAnnotations.add(marker);
+        mAnnotations.put(id, marker);
         return marker;
     }
 
     /**
      * <p>
+     * Updates a marker on this map. Does nothing if the marker is already added.
+     * </p>
+     *
+     * @param updatedMarker An updated marker object.
+     */
+    @UiThread
+    void updateMarker(@NonNull Marker updatedMarker) {
+        if (updatedMarker == null) {
+            Log.w(TAG, "marker was null, doing nothing");
+            return;
+        }
+
+        if (updatedMarker.getId() == -1) {
+            Log.w(TAG, "marker has an id of -1, possibly was not added yet, doing nothing");
+        }
+
+        ensureIconLoaded(updatedMarker);
+        mNativeMapView.updateMarker(updatedMarker);
+
+
+        int index = mAnnotations.indexOfKey(updatedMarker.getId());
+        if (index > -1) {
+            mAnnotations.setValueAt(index, updatedMarker);
+        }
+    }
+
+    /**
+     * <p>
      * Adds multiple markers to this map.
      * </p>
      * The marker's icon is rendered on the map at the location {@code Marker.position}.
@@ -1084,7 +1126,7 @@ public class MapView extends FrameLayout {
             m = markers.get(i);
             m.setId(ids[i]);
             m.setMapboxMap(mMapboxMap);
-            mAnnotations.add(m);
+            mAnnotations.put(ids[i], m);
         }
 
         return new ArrayList<>(markers);
@@ -1108,7 +1150,7 @@ public class MapView extends FrameLayout {
         long id = mNativeMapView.addPolyline(polyline);
         polyline.setId(id);
         polyline.setMapboxMap(mMapboxMap);
-        mAnnotations.add(polyline);
+        mAnnotations.put(id, polyline);
         return polyline;
     }
 
@@ -1139,7 +1181,7 @@ public class MapView extends FrameLayout {
             p = polylines.get(i);
             p.setId(ids[i]);
             p.setMapboxMap(mMapboxMap);
-            mAnnotations.add(p);
+            mAnnotations.put(ids[i], p);
         }
 
         return new ArrayList<>(polylines);
@@ -1163,7 +1205,7 @@ public class MapView extends FrameLayout {
         long id = mNativeMapView.addPolygon(polygon);
         polygon.setId(id);
         polygon.setMapboxMap(mMapboxMap);
-        mAnnotations.add(polygon);
+        mAnnotations.put(id, polygon);
         return polygon;
     }
 
@@ -1195,7 +1237,7 @@ public class MapView extends FrameLayout {
             p = polygons.get(i);
             p.setId(ids[i]);
             p.setMapboxMap(mMapboxMap);
-            mAnnotations.add(p);
+            mAnnotations.put(ids[i], p);
         }
 
         return new ArrayList<>(polygons);
@@ -1232,7 +1274,7 @@ public class MapView extends FrameLayout {
         }
         long id = annotation.getId();
         mNativeMapView.removeAnnotation(id);
-        mAnnotations.remove(annotation);
+        mAnnotations.delete(id);
     }
 
     /**
@@ -1261,10 +1303,10 @@ public class MapView extends FrameLayout {
     @UiThread
     void removeAllAnnotations() {
         int count = mAnnotations.size();
-        long[] ids = new long[mAnnotations.size()];
+        long[] ids = new long[count];
 
         for (int i = 0; i < count; i++) {
-            Annotation annotation = mAnnotations.get(i);
+            Annotation annotation = mAnnotations.valueAt(i);
             long id = annotation.getId();
             ids[i] = id;
             if (annotation instanceof Marker) {
@@ -1283,8 +1325,14 @@ public class MapView extends FrameLayout {
      * list will not update the map.
      */
     @NonNull
-    List<Annotation> getAllAnnotations() {
-        return new ArrayList<>(mAnnotations);
+    public List<Annotation> getAllAnnotations() {
+        List<Annotation> copyOfAnnotations = new ArrayList<>(mAnnotations.size());
+
+        for (int i = 0; i < mAnnotations.size(); i++) {
+            copyOfAnnotations.add(mAnnotations.valueAt(i));
+        }
+
+        return copyOfAnnotations;
     }
 
     private List<Marker> getMarkersInBounds(@NonNull LatLngBounds bbox) {
@@ -1304,7 +1352,7 @@ public class MapView extends FrameLayout {
         List<Marker> annotations = new ArrayList<>(ids.length);
         int count = mAnnotations.size();
         for (int i = 0; i < count; i++) {
-            Annotation annotation = mAnnotations.get(i);
+            Annotation annotation = mAnnotations.valueAt(i);
             if (annotation instanceof Marker && idsList.contains(annotation.getId())) {
                 annotations.add((Marker) annotation);
             }
@@ -1435,7 +1483,7 @@ public class MapView extends FrameLayout {
     private void adjustTopOffsetPixels() {
         int count = mAnnotations.size();
         for (int i = 0; i < count; i++) {
-            Annotation annotation = mAnnotations.get(i);
+            Annotation annotation = mAnnotations.valueAt(i);
             if (annotation instanceof Marker) {
                 Marker marker = (Marker) annotation;
                 marker.setTopOffsetPixels(
@@ -1454,7 +1502,7 @@ public class MapView extends FrameLayout {
     private void reloadMarkers() {
         int count = mAnnotations.size();
         for (int i = 0; i < count; i++) {
-            Annotation annotation = mAnnotations.get(i);
+            Annotation annotation = mAnnotations.valueAt(i);
             if (annotation instanceof Marker) {
                 Marker marker = (Marker) annotation;
                 mNativeMapView.removeAnnotation(annotation.getId());
@@ -1760,7 +1808,7 @@ public class MapView extends FrameLayout {
             if (newSelectedMarkerId >= 0) {
                 int count = mAnnotations.size();
                 for (int i = 0; i < count; i++) {
-                    Annotation annotation = mAnnotations.get(i);
+                    Annotation annotation = mAnnotations.valueAt(i);
                     if (annotation instanceof Marker) {
                         if (annotation.getId() == newSelectedMarkerId) {
                             if (selectedMarkers.isEmpty() || !selectedMarkers.contains(annotation)) {
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 3bb98f4d28..ce2f1e8981 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
@@ -537,6 +537,18 @@ public class MapboxMap {
     }
 
     /**
+     * <p>
+     * Updates a marker on this map. Does nothing if the marker is already added.
+     * </p>
+     *
+     * @param updatedMarker An updated marker object.
+     */
+    @UiThread
+    public void updateMarker(@NonNull Marker updatedMarker) {
+        mMapView.updateMarker(updatedMarker);
+    }
+
+    /**
      * Adds a polyline to this map.
      *
      * @param polylineOptions A polyline options object that defines how to render the polyline.
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java
index 6c388d9d8a..b7f583e943 100755
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java
@@ -395,6 +395,10 @@ final class NativeMapView {
         return nativeAddPolygons(mNativeMapViewPtr, polygon);
     }
 
+    public void updateMarker(Marker marker) {
+        nativeUpdateMarker(mNativeMapViewPtr, marker);
+    }
+
     public void removeAnnotation(long id) {
         nativeRemoveAnnotation(mNativeMapViewPtr, id);
     }
@@ -619,6 +623,8 @@ final class NativeMapView {
 
     private native long nativeAddMarker(long nativeMapViewPtr, Marker marker);
 
+    private native void nativeUpdateMarker(long nativeMapViewPtr, Marker marker);
+
     private native long[] nativeAddMarkers(long nativeMapViewPtr, List<Marker> markers);
 
     private native long nativeAddPolyline(long nativeMapViewPtr, Polyline polyline);
diff --git a/platform/android/src/jni.cpp b/platform/android/src/jni.cpp
index 00148562bb..db6bc73726 100755
--- a/platform/android/src/jni.cpp
+++ b/platform/android/src/jni.cpp
@@ -60,6 +60,7 @@ jfieldID iconIdId = nullptr;
 jclass markerClass = nullptr;
 jfieldID markerPositionId = nullptr;
 jfieldID markerIconId = nullptr;
+jfieldID markerIdId = nullptr;
 
 jclass polylineClass = nullptr;
 jfieldID polylineAlphaId = nullptr;
@@ -834,6 +835,47 @@ jlong JNICALL nativeAddMarker(JNIEnv *env, jobject obj, jlong nativeMapViewPtr,
     return nativeMapView->getMap().addPointAnnotation(mbgl::PointAnnotation(mbgl::LatLng(latitude, longitude), id));
 }
 
+void JNICALL nativeUpdateMarker(JNIEnv *env, jobject obj, jlong nativeMapViewPtr, jobject marker) {
+    mbgl::Log::Debug(mbgl::Event::JNI, "nativeUpdateMarker");
+    assert(nativeMapViewPtr != 0);
+    NativeMapView *nativeMapView = reinterpret_cast<NativeMapView *>(nativeMapViewPtr);
+
+    jlong markerId = env->GetLongField(marker, markerIdId);
+    if (env->ExceptionCheck()) {
+        env->ExceptionDescribe();
+    }
+
+    if (markerId == -1) {
+        return;
+    }
+
+    jobject position = env->GetObjectField(marker, markerPositionId);
+    if (env->ExceptionCheck()) {
+        env->ExceptionDescribe();
+    }
+
+    jobject icon = env->GetObjectField(marker, markerIconId);
+    if (env->ExceptionCheck()) {
+        env->ExceptionDescribe();
+    }
+
+    jstring jid = reinterpret_cast<jstring>(env->GetObjectField(icon, iconIdId));
+    std::string iconId = std_string_from_jstring(env, jid);
+
+    jdouble latitude = env->GetDoubleField(position, latLngLatitudeId);
+    if (env->ExceptionCheck()) {
+        env->ExceptionDescribe();
+    }
+
+    jdouble longitude = env->GetDoubleField(position, latLngLongitudeId);
+    if (env->ExceptionCheck()) {
+        env->ExceptionDescribe();
+    }
+
+    // Because Java only has int, not unsigned int, we need to bump the annotation id up to a long.
+    nativeMapView->getMap().updatePointAnnotation(markerId, mbgl::PointAnnotation(mbgl::LatLng(latitude, longitude), iconId));
+}
+
 jlongArray JNICALL nativeAddMarkers(JNIEnv *env, jobject obj, jlong nativeMapViewPtr, jobject jlist) {
     mbgl::Log::Debug(mbgl::Event::JNI, "nativeAddMarkers");
     assert(nativeMapViewPtr != 0);
@@ -1685,6 +1727,12 @@ extern "C" JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void *reserved) {
         return JNI_ERR;
     }
 
+    markerIdId = env->GetFieldID(markerClass, "id", "J");
+    if (markerIdId == nullptr) {
+        env->ExceptionDescribe();
+        return JNI_ERR;
+    }
+
     polylineClass = env->FindClass("com/mapbox/mapboxsdk/annotations/Polyline");
     if (polylineClass == nullptr) {
         env->ExceptionDescribe();
@@ -2044,6 +2092,8 @@ extern "C" JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void *reserved) {
          reinterpret_cast<void *>(&nativeAddPolygon)},
         {"nativeAddPolygons", "(JLjava/util/List;)[J",
          reinterpret_cast<void *>(&nativeAddPolygons)},
+        {"nativeUpdateMarker", "(JLcom/mapbox/mapboxsdk/annotations/Marker;)V",
+        reinterpret_cast<void *>(&nativeUpdateMarker)} ,
         {"nativeRemoveAnnotation", "(JJ)V", reinterpret_cast<void *>(&nativeRemoveAnnotation)},
         {"nativeRemoveAnnotations", "(J[J)V", reinterpret_cast<void *>(&nativeRemoveAnnotations)},
         {"nativeGetAnnotationsInBounds", "(JLcom/mapbox/mapboxsdk/geometry/LatLngBounds;)[J",
@@ -2304,6 +2354,7 @@ extern "C" JNIEXPORT void JNICALL JNI_OnUnload(JavaVM *vm, void *reserved) {
     markerClass = nullptr;
     markerPositionId = nullptr;
     markerIconId = nullptr;
+    markerIdId = nullptr;
 
     env->DeleteGlobalRef(polylineClass);
     polylineClass = nullptr;