From 6fce6bb6d2f72abbddd02ec0dd0291df7d48fe47 Mon Sep 17 00:00:00 2001 From: Tobrun Van Nuland Date: Fri, 28 Jul 2017 18:58:27 +0200 Subject: [android] - optimise icon management --- .../mapbox/mapboxsdk/maps/AnnotationManager.java | 7 + .../com/mapbox/mapboxsdk/maps/IconManager.java | 52 +++++--- .../com/mapbox/mapboxsdk/maps/NativeMapView.java | 9 ++ .../mapbox/mapboxsdk/maps/IconManagerResolver.java | 42 ++++++ .../mapboxsdk/testapp/annotations/IconTest.java | 146 +++++++++++++++++++++ platform/android/src/native_map_view.cpp | 6 + platform/android/src/native_map_view.hpp | 2 + 7 files changed, 248 insertions(+), 16 deletions(-) create mode 100644 platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/maps/IconManagerResolver.java create mode 100644 platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/annotations/IconTest.java diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/AnnotationManager.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/AnnotationManager.java index edf448ab4f..c09c926eb5 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/AnnotationManager.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/AnnotationManager.java @@ -118,6 +118,9 @@ class AnnotationManager { if (marker instanceof MarkerView) { markerViewManager.removeMarkerView((MarkerView) marker); + } else { + // do icon cleanup + iconManager.iconCleanup(marker.getIcon()); } } else { // instanceOf Polygon/Polyline @@ -137,6 +140,8 @@ class AnnotationManager { if (marker instanceof MarkerView) { markerViewManager.removeMarkerView((MarkerView) marker); + } else { + iconManager.iconCleanup(marker.getIcon()); } } else { // instanceOf Polygon/Polyline @@ -159,6 +164,8 @@ class AnnotationManager { marker.hideInfoWindow(); if (marker instanceof MarkerView) { markerViewManager.removeMarkerView((MarkerView) marker); + } else { + iconManager.iconCleanup(marker.getIcon()); } } else { // instanceOf Polygon/Polyline diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/IconManager.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/IconManager.java index 18eecfd9c3..affbf48267 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/IconManager.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/IconManager.java @@ -1,16 +1,17 @@ package com.mapbox.mapboxsdk.maps; import android.graphics.Bitmap; +import android.os.Build; import com.mapbox.mapboxsdk.Mapbox; import com.mapbox.mapboxsdk.annotations.Icon; import com.mapbox.mapboxsdk.annotations.IconFactory; import com.mapbox.mapboxsdk.annotations.Marker; import com.mapbox.mapboxsdk.annotations.MarkerView; -import com.mapbox.mapboxsdk.exceptions.IconBitmapChangedException; -import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; /** * Responsible for managing icons added to the Map. @@ -25,15 +26,14 @@ import java.util.List; */ class IconManager { - private NativeMapView nativeMapView; - private List icons; + private final Map iconMap = new HashMap<>(); + private NativeMapView nativeMapView; private int highestIconWidth; private int highestIconHeight; IconManager(NativeMapView nativeMapView) { this.nativeMapView = nativeMapView; - this.icons = new ArrayList<>(); // load transparent icon for MarkerView to trace actual markers, see #6352 loadIcon(IconFactory.recreate(IconFactory.ICON_MARKERVIEW_ID, IconFactory.ICON_MARKERVIEW_BITMAP)); } @@ -83,13 +83,13 @@ class IconManager { } private void addIcon(Icon icon, boolean addIconToMap) { - if (!icons.contains(icon)) { - icons.add(icon); + if (!iconMap.keySet().contains(icon)) { + iconMap.put(icon, 1); if (addIconToMap) { loadIcon(icon); } } else { - validateIconChanged(icon); + iconMap.put(icon, iconMap.get(icon) + 1); } } @@ -121,18 +121,11 @@ class IconManager { } void reloadIcons() { - for (Icon icon : icons) { + for (Icon icon : iconMap.keySet()) { loadIcon(icon); } } - private void validateIconChanged(Icon icon) { - Icon oldIcon = icons.get(icons.indexOf(icon)); - if (!oldIcon.getBitmap().sameAs(icon.getBitmap())) { - throw new IconBitmapChangedException(); - } - } - void ensureIconLoaded(Marker marker, MapboxMap mapboxMap) { Icon icon = marker.getIcon(); if (icon == null) { @@ -149,4 +142,31 @@ class IconManager { marker.setTopOffsetPixels(getTopOffsetPixelsForIcon(icon)); } } + + public void iconCleanup(Icon icon) { + int refCounter = iconMap.get(icon) - 1; + if (refCounter == 0) { + remove(icon); + } else { + updateIconRefCounter(icon, refCounter); + } + } + + private void remove(Icon icon) { + nativeMapView.removeAnnotationIcon(icon.getId()); + iconMap.remove(icon); + if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { + recycleBitmap(icon.getBitmap()); + } + } + + private void updateIconRefCounter(Icon icon, int refCounter) { + iconMap.put(icon, refCounter); + } + + private void recycleBitmap(Bitmap bitmap) { + if (!bitmap.isRecycled()) { + bitmap.recycle(); + } + } } 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 ddeb4b326e..b2d49bdbf8 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 @@ -507,6 +507,13 @@ final class NativeMapView { nativeAddAnnotationIcon(symbol, width, height, scale, pixels); } + public void removeAnnotationIcon(String symbol) { + if (isDestroyedOn("removeAnnotationIcon")) { + return; + } + nativeRemoveAnnotationIcon(symbol); + } + public void setVisibleCoordinateBounds(LatLng[] coordinates, RectF padding, double direction, long duration) { if (isDestroyedOn("setVisibleCoordinateBounds")) { return; @@ -990,6 +997,8 @@ final class NativeMapView { private native void nativeAddAnnotationIcon(String symbol, int width, int height, float scale, byte[] pixels); + private native void nativeRemoveAnnotationIcon(String symbol); + private native void nativeSetVisibleCoordinateBounds(LatLng[] coordinates, RectF padding, double direction, long duration); diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/maps/IconManagerResolver.java b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/maps/IconManagerResolver.java new file mode 100644 index 0000000000..3e226a7ec5 --- /dev/null +++ b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/maps/IconManagerResolver.java @@ -0,0 +1,42 @@ +package com.mapbox.mapboxsdk.maps; + +import com.mapbox.mapboxsdk.annotations.Icon; + +import java.lang.reflect.Field; +import java.util.HashMap; +import java.util.Map; + +import timber.log.Timber; + +public class IconManagerResolver { + + private IconManager iconManager; + + public IconManagerResolver(MapboxMap mapboxMap) { + try { + Field annotationManagerField = MapboxMap.class.getDeclaredField("annotationManager"); + annotationManagerField.setAccessible(true); + AnnotationManager annotationManager = (AnnotationManager) annotationManagerField.get(mapboxMap); + + Field iconManagerField = AnnotationManager.class.getDeclaredField("iconManager"); + iconManagerField.setAccessible(true); + iconManager = (IconManager) iconManagerField.get(annotationManager); + } catch (Exception exception) { + Timber.e(exception, "Could not create IconManagerResolver, unable to reflect."); + } + } + + @SuppressWarnings("unchecked") + public Map getIconMap() { + try { + Field field = IconManager.class.getDeclaredField("iconMap"); + field.setAccessible(true); + return (Map) field.get(iconManager); + } catch (NoSuchFieldException exception) { + Timber.e(exception, "Could not getIconMap, unable to reflect."); + } catch (IllegalAccessException exception) { + Timber.e(exception, "Could not getIconMap, unable to reflect."); + } + return new HashMap<>(); + } +} diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/annotations/IconTest.java b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/annotations/IconTest.java new file mode 100644 index 0000000000..33a946d0a1 --- /dev/null +++ b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/annotations/IconTest.java @@ -0,0 +1,146 @@ +package com.mapbox.mapboxsdk.testapp.annotations; + +import android.app.Activity; +import android.support.v4.content.res.ResourcesCompat; + +import com.mapbox.mapboxsdk.annotations.Icon; +import com.mapbox.mapboxsdk.annotations.IconFactory; +import com.mapbox.mapboxsdk.annotations.Marker; +import com.mapbox.mapboxsdk.annotations.MarkerOptions; +import com.mapbox.mapboxsdk.geometry.LatLng; +import com.mapbox.mapboxsdk.maps.IconManagerResolver; +import com.mapbox.mapboxsdk.maps.MapboxMap; +import com.mapbox.mapboxsdk.testapp.R; +import com.mapbox.mapboxsdk.testapp.activity.BaseActivityTest; +import com.mapbox.mapboxsdk.testapp.activity.espresso.EspressoTestActivity; +import com.mapbox.mapboxsdk.testapp.utils.IconUtils; + +import org.junit.Before; +import org.junit.Test; + +import java.util.Map; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.assertNull; +import static junit.framework.Assert.assertTrue; + +/** + * Tests integration between Icons and Markers + */ +public class IconTest extends BaseActivityTest { + + private Map iconMap; + + @Before + public void beforeTest() { + super.beforeTest(); + iconMap = new IconManagerResolver(getMapboxMap()).getIconMap(); + } + + @Test + public void testEmpty() throws Exception { + assertTrue(iconMap.isEmpty()); + } + + @Test + public void testAddSameIconMarker() throws Exception { + Icon defaultMarker = IconFactory.getInstance(rule.getActivity()).defaultMarker(); + getMapboxMap().addMarker(new MarkerOptions().position(new LatLng())); + getMapboxMap().addMarker(new MarkerOptions().position(new LatLng(1, 1))); + assertEquals(1, iconMap.size()); + assertEquals(2, iconMap.get(defaultMarker), 0); + } + + @Test + public void testAddDifferentIconMarker() throws Exception { + Icon icon = IconFactory.getInstance(rule.getActivity()).fromResource(R.drawable.mapbox_logo_icon); + getMapboxMap().addMarker(new MarkerOptions().icon(icon).position(new LatLng())); + getMapboxMap().addMarker(new MarkerOptions().position(new LatLng(1, 1))); + assertEquals(iconMap.size(), 2); + assertTrue(iconMap.containsKey(icon)); + assertTrue(iconMap.get(icon) == 1); + } + + @Test + public void testAddRemoveIconMarker() throws Exception { + MapboxMap mapboxMap = getMapboxMap(); + + Icon icon = IconFactory.getInstance(rule.getActivity()).fromResource(R.drawable.mapbox_logo_icon); + Marker marker = mapboxMap.addMarker(new MarkerOptions().icon(icon).position(new LatLng())); + mapboxMap.addMarker(new MarkerOptions().position(new LatLng(1, 1))); + assertEquals(iconMap.size(), 2); + assertTrue(iconMap.containsKey(icon)); + assertTrue(iconMap.get(icon) == 1); + + mapboxMap.removeMarker(marker); + assertEquals(iconMap.size(), 1); + assertFalse(iconMap.containsKey(icon)); + } + + @Test + public void testAddRemoveDefaultMarker() throws Exception { + MapboxMap mapboxMap = getMapboxMap(); + + Marker marker = mapboxMap.addMarker(new MarkerOptions().position(new LatLng(1, 1))); + assertEquals(iconMap.size(), 1); + + mapboxMap.removeMarker(marker); + assertEquals(iconMap.size(), 0); + + mapboxMap.addMarker(new MarkerOptions().position(new LatLng())); + assertEquals(iconMap.size(), 1); + } + + @Test + public void testAddRemoveMany() throws Exception { + Activity activity = rule.getActivity(); + MapboxMap mapboxMap = getMapboxMap(); + IconFactory iconFactory = IconFactory.getInstance(activity); + + // add 2 default icon markers + Marker defaultMarkerOne = mapboxMap.addMarker(new MarkerOptions().position(new LatLng(1, 1))); + Marker defaultMarkerTwo = mapboxMap.addMarker(new MarkerOptions().position(new LatLng(2, 1))); + + // add 4 unique icon markers + mapboxMap.addMarker(new MarkerOptions() + .icon(iconFactory.fromResource(R.drawable.mapbox_logo_icon)) + .position(new LatLng(3, 1)) + ); + mapboxMap.addMarker(new MarkerOptions() + .icon(iconFactory.fromResource(R.drawable.mapbox_compass_icon)) + .position(new LatLng(4, 1)) + ); + mapboxMap.addMarker(new MarkerOptions() + .icon(IconUtils.drawableToIcon(activity, R.drawable.ic_stars, + ResourcesCompat.getColor(activity.getResources(), + R.color.blueAccent, activity.getTheme()))) + .position(new LatLng(5, 1)) + ); + mapboxMap.addMarker(new MarkerOptions() + .icon(iconFactory.fromResource(R.drawable.ic_android)) + .position(new LatLng(6, 1)) + ); + + assertEquals("Amount of icons should match 5", 5, iconMap.size()); + assertEquals("Refcounter of default marker should match 2", 2, iconMap.get(iconFactory.defaultMarker()), 0); + + mapboxMap.removeMarker(defaultMarkerOne); + + assertEquals("Amount of icons should match 5", 5, iconMap.size()); + assertEquals("Refcounter of default marker should match 1", 1, iconMap.get(iconFactory.defaultMarker()), 0); + + mapboxMap.removeMarker(defaultMarkerTwo); + + assertEquals("Amount of icons should match 4", 4, iconMap.size()); + assertNull("DefaultMarker shouldn't exist anymore", iconMap.get(iconFactory.defaultMarker())); + + mapboxMap.clear(); + assertEquals("Amount of icons should match 0", 0, iconMap.size()); + } + + @Override + protected Class getActivityClass() { + return EspressoTestActivity.class; + } +} diff --git a/platform/android/src/native_map_view.cpp b/platform/android/src/native_map_view.cpp index fa9e90ddc6..58c39355d1 100755 --- a/platform/android/src/native_map_view.cpp +++ b/platform/android/src/native_map_view.cpp @@ -716,6 +716,11 @@ void NativeMapView::addAnnotationIcon(JNIEnv& env, jni::String symbol, jint w, j symbolName, std::move(premultipliedImage), float(scale))); } +void NativeMapView::removeAnnotationIcon(JNIEnv& env, jni::String symbol) { + const std::string symbolName = jni::Make(env, symbol); + map->removeAnnotationImage(symbolName); +} + jdouble NativeMapView::getTopOffsetPixelsForAnnotationSymbol(JNIEnv& env, jni::String symbolName) { return map->getTopOffsetPixelsForAnnotationImage(jni::Make(env, symbolName)); } @@ -1523,6 +1528,7 @@ void NativeMapView::registerNative(jni::JNIEnv& env) { METHOD(&NativeMapView::updatePolygon, "nativeUpdatePolygon"), METHOD(&NativeMapView::removeAnnotations, "nativeRemoveAnnotations"), METHOD(&NativeMapView::addAnnotationIcon, "nativeAddAnnotationIcon"), + METHOD(&NativeMapView::removeAnnotationIcon, "nativeRemoveAnnotationIcon"), METHOD(&NativeMapView::getTopOffsetPixelsForAnnotationSymbol, "nativeGetTopOffsetPixelsForAnnotationSymbol"), METHOD(&NativeMapView::getTransitionDuration, "nativeGetTransitionDuration"), METHOD(&NativeMapView::setTransitionDuration, "nativeSetTransitionDuration"), diff --git a/platform/android/src/native_map_view.hpp b/platform/android/src/native_map_view.hpp index ba1545728a..ec82a36a5c 100755 --- a/platform/android/src/native_map_view.hpp +++ b/platform/android/src/native_map_view.hpp @@ -199,6 +199,8 @@ public: void addAnnotationIcon(JNIEnv&, jni::String, jint, jint, jfloat, jni::Array); + void removeAnnotationIcon(JNIEnv&, jni::String); + jni::jdouble getTopOffsetPixelsForAnnotationSymbol(JNIEnv&, jni::String); jni::jlong getTransitionDuration(JNIEnv&); -- cgit v1.2.1