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

Commit

Permalink
[android] - optimise icon management
Browse files Browse the repository at this point in the history
  • Loading branch information
tobrun committed Aug 10, 2017
1 parent 9238dd4 commit 6fce6bb
Show file tree
Hide file tree
Showing 7 changed files with 248 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ void removeAnnotation(@NonNull Annotation annotation) {

if (marker instanceof MarkerView) {
markerViewManager.removeMarkerView((MarkerView) marker);
} else {
// do icon cleanup
iconManager.iconCleanup(marker.getIcon());
}
} else {
// instanceOf Polygon/Polyline
Expand All @@ -137,6 +140,8 @@ void removeAnnotations(@NonNull List<? extends Annotation> annotationList) {

if (marker instanceof MarkerView) {
markerViewManager.removeMarkerView((MarkerView) marker);
} else {
iconManager.iconCleanup(marker.getIcon());
}
} else {
// instanceOf Polygon/Polyline
Expand All @@ -159,6 +164,8 @@ void removeAnnotations() {
marker.hideInfoWindow();
if (marker instanceof MarkerView) {
markerViewManager.removeMarkerView((MarkerView) marker);
} else {
iconManager.iconCleanup(marker.getIcon());
}
} else {
// instanceOf Polygon/Polyline
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -25,15 +26,14 @@
*/
class IconManager {

private NativeMapView nativeMapView;
private List<Icon> icons;
private final Map<Icon, Integer> 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));
}
Expand Down Expand Up @@ -83,13 +83,13 @@ private void addIcon(Icon icon) {
}

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);
}
}

Expand Down Expand Up @@ -121,18 +121,11 @@ private void loadIcon(Icon icon) {
}

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) {
Expand All @@ -149,4 +142,31 @@ private void setTopOffsetPixels(Marker marker, MapboxMap mapboxMap, Icon icon) {
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();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,13 @@ public void addAnnotationIcon(String symbol, int width, int height, float scale,
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;
Expand Down Expand Up @@ -990,6 +997,8 @@ private native void nativeInitialize(NativeMapView 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);

Expand Down
Original file line number Diff line number Diff line change
@@ -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<Icon, Integer> getIconMap() {
try {
Field field = IconManager.class.getDeclaredField("iconMap");
field.setAccessible(true);
return (Map<Icon, Integer>) 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<>();
}
}
Original file line number Diff line number Diff line change
@@ -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<Icon, Integer> 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;
}
}
6 changes: 6 additions & 0 deletions platform/android/src/native_map_view.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string>(env, symbol);
map->removeAnnotationImage(symbolName);
}

jdouble NativeMapView::getTopOffsetPixelsForAnnotationSymbol(JNIEnv& env, jni::String symbolName) {
return map->getTopOffsetPixelsForAnnotationImage(jni::Make<std::string>(env, symbolName));
}
Expand Down Expand Up @@ -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"),
Expand Down
2 changes: 2 additions & 0 deletions platform/android/src/native_map_view.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,8 @@ class NativeMapView : public RendererBackend, public MapObserver {

void addAnnotationIcon(JNIEnv&, jni::String, jint, jint, jfloat, jni::Array<jbyte>);

void removeAnnotationIcon(JNIEnv&, jni::String);

jni::jdouble getTopOffsetPixelsForAnnotationSymbol(JNIEnv&, jni::String);

jni::jlong getTransitionDuration(JNIEnv&);
Expand Down

0 comments on commit 6fce6bb

Please sign in to comment.