[android] - harden orientation changes (#9128)

5 files changed, 127 insertions, 98 deletions

diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/annotations/Icon.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/annotations/Icon.java
index b1a05ec436..2ee17c227d 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/annotations/Icon.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/annotations/Icon.java
@@ -1,15 +1,18 @@
 package com.mapbox.mapboxsdk.annotations;
 
 import android.graphics.Bitmap;
+import android.util.DisplayMetrics;
 
-import com.mapbox.mapboxsdk.maps.MapView;
+import java.nio.ByteBuffer;
 
 /**
- * Icon is the visual representation of a {@link Marker} on a {@link MapView}.
+ * Icon is the visual representation of a Marker on a MapView.
  *
  * @see Marker
+ * @see IconFactory
  */
 public class Icon {
+
   private Bitmap mBitmap;
   private String mId;
 
@@ -19,29 +22,67 @@ public class Icon {
   }
 
   /**
-   * {@link String} identifier for this {@link Icon}.
+   * String identifier for this icon.
    *
-   * @return {@link String} identifier for this {@link Icon}.
+   * @return String identifier for this icon.
    */
   public String getId() {
     return mId;
   }
 
   /**
-   * Get the {@link Bitmap} being used for this {@link Icon}.
+   * Get the bitmap being used for this icon.
    *
-   * @return The {@link Bitmap} being used for the {@link Icon}.
+   * @return The bitmap being used for the icon.
    */
   public Bitmap getBitmap() {
+    if (mBitmap.getConfig() != Bitmap.Config.ARGB_8888) {
+      mBitmap = mBitmap.copy(Bitmap.Config.ARGB_8888, false);
+    }
     return mBitmap;
   }
 
   /**
-   * Compares this {@link Icon} object with another {@link Icon} and determines if they match.
+   * Get the icon bitmap scale.
+   * <p>
+   * Requires the bitmap to be set before calling this method.
+   * </p>
+   *
+   * @return the scale of the bitmap
+   */
+  public float getScale() {
+    if (mBitmap == null) {
+      throw new IllegalStateException("Required to set a Icon before calling getScale");
+    }
+    float density = mBitmap.getDensity();
+    if (density == Bitmap.DENSITY_NONE) {
+      density = DisplayMetrics.DENSITY_DEFAULT;
+    }
+    return density / DisplayMetrics.DENSITY_DEFAULT;
+  }
+
+  /**
+   * Get the icon bitmap bytes.
+   * <p>
+   * Requires the bitmap to be set before calling this method.
+   * </p>
    *
-   * @param object Another {@link Icon} to compare with this object.
-   * @return True if the {@link Icon} being passed in matches this {@link Icon} object. Else,
-   * false.
+   * @return the bytes of the bitmap
+   */
+  public byte[] toBytes() {
+    if (mBitmap == null) {
+      throw new IllegalStateException("Required to set a Icon before calling toBytes");
+    }
+    ByteBuffer buffer = ByteBuffer.allocate(mBitmap.getRowBytes() * mBitmap.getHeight());
+    mBitmap.copyPixelsToBuffer(buffer);
+    return buffer.array();
+  }
+
+  /**
+   * Compares this icon object with another icon and determines if they match.
+   *
+   * @param object Another iconi to compare with this object.
+   * @return True if the icon being passed in matches this icon object. Else, false.
    */
   @Override
   public boolean equals(Object object) {
@@ -53,11 +94,7 @@ public class Icon {
     }
 
     Icon icon = (Icon) object;
-
-    if (!mBitmap.equals(icon.mBitmap)) {
-      return false;
-    }
-    return mId.equals(icon.mId);
+    return mBitmap.equals(icon.mBitmap) && mId.equals(icon.mId);
   }
 
   /**
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/annotations/MarkerView.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/annotations/MarkerView.java
index 30336d4ebd..56e8cc4ce2 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/annotations/MarkerView.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/annotations/MarkerView.java
@@ -3,6 +3,7 @@ package com.mapbox.mapboxsdk.annotations;
 import android.support.annotation.FloatRange;
 import android.support.annotation.Nullable;
 
+import com.mapbox.mapboxsdk.Mapbox;
 import com.mapbox.mapboxsdk.constants.MapboxConstants;
 import com.mapbox.mapboxsdk.geometry.LatLng;
 import com.mapbox.mapboxsdk.maps.MapboxMap;
@@ -361,6 +362,9 @@ public class MarkerView extends Marker {
    */
   @Override
   public Icon getIcon() {
+    if (markerViewIcon == null) {
+      setIcon(IconFactory.getInstance(Mapbox.getApplicationContext()).defaultMarkerView());
+    }
     return markerViewIcon;
   }
 
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 c9d81a88bc..9f4171aee8 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,15 +1,14 @@
 package com.mapbox.mapboxsdk.maps;
 
 import android.graphics.Bitmap;
-import android.util.DisplayMetrics;
 
+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.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -41,118 +40,109 @@ class IconManager {
 
   Icon loadIconForMarker(Marker marker) {
     Icon icon = marker.getIcon();
-
-    // calculating average before adding
-    int iconSize = icons.size() + 1;
-
-    // TODO replace former if case with anchor implementation,
-    // current workaround for having extra pixels is diving height by 2
     if (icon == null) {
-      icon = IconFactory.getInstance(nativeMapView.getContext()).defaultMarker();
-      Bitmap bitmap = icon.getBitmap();
-      averageIconHeight = averageIconHeight + (bitmap.getHeight() / 2 - averageIconHeight) / iconSize;
-      averageIconWidth = averageIconWidth + (bitmap.getWidth() - averageIconWidth) / iconSize;
-      marker.setIcon(icon);
-    } else {
-      Bitmap bitmap = icon.getBitmap();
-      averageIconHeight = averageIconHeight + (bitmap.getHeight() - averageIconHeight) / iconSize;
-      averageIconWidth = averageIconWidth + (bitmap.getWidth() - averageIconWidth) / iconSize;
-    }
-
-    if (!icons.contains(icon)) {
-      icons.add(icon);
-      loadIcon(icon);
+      // TODO replace with anchor implementation, we are faking an anchor by adding extra pixels and diving height by 2
+      // TODO we can move this code afterwards to getIcon as with MarkerView.getIcon
+      icon = loadDefaultIconForMarker(marker);
     } else {
-      Icon oldIcon = icons.get(icons.indexOf(icon));
-      if (!oldIcon.getBitmap().sameAs(icon.getBitmap())) {
-        throw new IconBitmapChangedException();
-      }
+      updateAverageIconSize(icon);
     }
+    addIcon(icon);
     return icon;
   }
 
-  Icon loadIconForMarkerView(MarkerView marker) {
+  void loadIconForMarkerView(MarkerView marker) {
     Icon icon = marker.getIcon();
-    int iconSize = icons.size() + 1;
-    if (icon == null) {
-      icon = IconFactory.getInstance(nativeMapView.getContext()).defaultMarkerView();
-      marker.setIcon(icon);
-    }
     Bitmap bitmap = icon.getBitmap();
-    averageIconHeight = averageIconHeight + (bitmap.getHeight() - averageIconHeight) / iconSize;
-    averageIconWidth = averageIconWidth + (bitmap.getWidth() - averageIconWidth) / iconSize;
-    if (!icons.contains(icon)) {
-      icons.add(icon);
-    } else {
-      Icon oldIcon = icons.get(icons.indexOf(icon));
-      if (!oldIcon.getBitmap().sameAs(icon.getBitmap())) {
-        throw new IconBitmapChangedException();
-      }
-    }
-    return icon;
+    updateAverageIconSize(bitmap);
+    addIcon(icon, false);
   }
 
   int getTopOffsetPixelsForIcon(Icon icon) {
     return (int) (nativeMapView.getTopOffsetPixelsForAnnotationSymbol(icon.getId()) * nativeMapView.getPixelRatio());
   }
 
-  void loadIcon(Icon icon) {
+  int getAverageIconHeight() {
+    return averageIconHeight;
+  }
+
+  int getAverageIconWidth() {
+    return averageIconWidth;
+  }
+
+  private Icon loadDefaultIconForMarker(Marker marker) {
+    Icon icon = IconFactory.getInstance(Mapbox.getApplicationContext()).defaultMarker();
     Bitmap bitmap = icon.getBitmap();
-    String id = icon.getId();
-    if (bitmap.getConfig() != Bitmap.Config.ARGB_8888) {
-      bitmap = bitmap.copy(Bitmap.Config.ARGB_8888, false);
-    }
-    ByteBuffer buffer = ByteBuffer.allocate(bitmap.getRowBytes() * bitmap.getHeight());
-    bitmap.copyPixelsToBuffer(buffer);
+    updateAverageIconSize(bitmap.getWidth(), bitmap.getHeight() / 2);
+    marker.setIcon(icon);
+    return icon;
+  }
+
+  private void addIcon(Icon icon) {
+    addIcon(icon, true);
+  }
 
-    float density = bitmap.getDensity();
-    if (density == Bitmap.DENSITY_NONE) {
-      density = DisplayMetrics.DENSITY_DEFAULT;
+  private void addIcon(Icon icon, boolean addIconToMap) {
+    if (!icons.contains(icon)) {
+      icons.add(icon);
+      if (addIconToMap) {
+        loadIcon(icon);
+      }
+    } else {
+      validateIconChanged(icon);
     }
-    float scale = density / DisplayMetrics.DENSITY_DEFAULT;
-    nativeMapView.addAnnotationIcon(
-      id,
+  }
+
+  private void updateAverageIconSize(Icon icon) {
+    updateAverageIconSize(icon.getBitmap());
+  }
+
+  private void updateAverageIconSize(Bitmap bitmap) {
+    updateAverageIconSize(bitmap.getWidth(), bitmap.getHeight());
+  }
+
+  private void updateAverageIconSize(int width, int height) {
+    int iconSize = icons.size() + 1;
+    averageIconHeight = averageIconHeight + (height - averageIconHeight) / iconSize;
+    averageIconWidth = averageIconWidth + (width - averageIconWidth) / iconSize;
+  }
+
+  private void loadIcon(Icon icon) {
+    Bitmap bitmap = icon.getBitmap();
+    nativeMapView.addAnnotationIcon(icon.getId(),
       bitmap.getWidth(),
       bitmap.getHeight(),
-      scale, buffer.array());
+      icon.getScale(),
+      icon.toBytes());
   }
 
   void reloadIcons() {
-    int count = icons.size();
-    for (int i = 0; i < count; i++) {
-      Icon icon = icons.get(i);
+    for (Icon icon : icons) {
       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) {
-      icon = IconFactory.getInstance(nativeMapView.getContext()).defaultMarker();
-      marker.setIcon(icon);
-    }
-    if (!icons.contains(icon)) {
-      icons.add(icon);
-      loadIcon(icon);
-    } else {
-      Icon oldIcon = icons.get(icons.indexOf(icon));
-      if (!oldIcon.getBitmap().sameAs(icon.getBitmap())) {
-        throw new IconBitmapChangedException();
-      }
+      icon = loadDefaultIconForMarker(marker);
     }
+    addIcon(icon);
+    setTopOffsetPixels(marker, mapboxMap, icon);
+  }
 
+  private void setTopOffsetPixels(Marker marker, MapboxMap mapboxMap, Icon icon) {
     // 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) mapboxMap.getAnnotation(marker.getId()) : null;
     if (previousMarker == null || previousMarker.getIcon() == null || previousMarker.getIcon() != marker.getIcon()) {
       marker.setTopOffsetPixels(getTopOffsetPixelsForIcon(icon));
     }
   }
-
-  int getAverageIconHeight() {
-    return averageIconHeight;
-  }
-
-  int getAverageIconWidth() {
-    return averageIconWidth;
-  }
 }
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 02eeb25f83..63f2e653cc 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
@@ -897,10 +897,6 @@ final class NativeMapView {
     return pixelRatio;
   }
 
-  public Context getContext() {
-    return mapView.getContext();
-  }
-
   //
   // Callbacks
   //
diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/annotations/IconTest.java b/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/annotations/IconTest.java
index 5f6f6b6c6d..1c259af2d0 100644
--- a/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/annotations/IconTest.java
+++ b/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/annotations/IconTest.java
@@ -9,6 +9,7 @@ import org.mockito.MockitoAnnotations;
 
 import static junit.framework.Assert.assertEquals;
 import static junit.framework.Assert.assertNotSame;
+import static org.mockito.Mockito.when;
 
 public class IconTest {
 
@@ -18,6 +19,7 @@ public class IconTest {
   @Before
   public void beforeTest() {
     MockitoAnnotations.initMocks(this);
+    when(bitmap.getConfig()).thenReturn(Bitmap.Config.ARGB_8888);
   }
 
   @Test