[core] [android] Fix InfoWindow topOffsetPixels adjustment when switc…

…hing styles

Fixes #2599
Make getTopOffsetPixelsForAnnotationSymbol private
Also implements MapChangeWillStartLoadingMap, MapChangeDidFinishLoadingMap
Paritally implements #630

android/cpp/jni.cpp

@@ -456,6 +456,13 @@ void JNICALL nativePause(JNIEnv *env, jobject obj, jlong nativeMapViewPtr) {
     nativeMapView->pause();
 }
 
+jboolean JNICALL nativeIsPaused(JNIEnv *env, jobject obj, jlong nativeMapViewPtr) {
+    mbgl::Log::Debug(mbgl::Event::JNI, "nativeIsPaused");
+    assert(nativeMapViewPtr != 0);
+    NativeMapView *nativeMapView = reinterpret_cast<NativeMapView *>(nativeMapViewPtr);
+    return nativeMapView->getMap().isPaused();
+}
+
 void JNICALL nativeResume(JNIEnv *env, jobject obj, jlong nativeMapViewPtr) {
     mbgl::Log::Debug(mbgl::Event::JNI, "nativeResume");
     assert(nativeMapViewPtr != 0);
@@ -1690,6 +1697,7 @@ extern "C" JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void *reserved) {
          reinterpret_cast<void *>(&nativeCreateSurface)},
         {"nativeDestroySurface", "(J)V", reinterpret_cast<void *>(&nativeDestroySurface)},
         {"nativePause", "(J)V", reinterpret_cast<void *>(&nativePause)},
+        {"nativeIsPaused", "(J)Z", reinterpret_cast<void *>(&nativeIsPaused)},
         {"nativeResume", "(J)V", reinterpret_cast<void *>(&nativeResume)},
         {"nativeUpdate", "(J)V", reinterpret_cast<void *>(&nativeUpdate)},
         {"nativeRenderSync", "(J)V", reinterpret_cast<void *>(&nativeRenderSync)},

android/java/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/annotations/InfoWindow.java

@@ -81,14 +81,8 @@ public InfoWindow open(Marker object, LatLng position, int offsetX, int offsetY)
 
         // Calculate default Android x,y coordinate
         PointF coords = mMapView.toScreenLocation(position);
-        float x = coords.x - (mView.getMeasuredWidth() / 2);
-        float y = (float) mMapView.getTopOffsetPixelsForAnnotationSymbol(object.getSprite());
-        y = y * mMapView.getScreenDensity();
-        y += coords.y - mView.getMeasuredHeight();
-
-// if getTopOffsetPixelsForAnnotationSymbol lands
-//        float x = coords.x - (mView.getMeasuredWidth() / 2) + offsetX;
-//        float y = coords.y - mView.getMeasuredHeight() + offsetY;
+        float x = coords.x - (mView.getMeasuredWidth() / 2) + offsetX;
+        float y = coords.y - mView.getMeasuredHeight() + offsetY;
 
         // get right/left popup window
         float right = x + mView.getMeasuredWidth();

android/java/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/annotations/Marker.java

@@ -15,14 +15,15 @@ public class Marker extends Annotation {
     private boolean flat;
     private float infoWindowAnchorU;
     private float infoWindowAnchorV;
-    LatLng position;
+    private LatLng position;
     private float rotation;
     private String snippet;
-    String sprite = "default_marker";
+    private String sprite = "default_marker";
     private String title;
     private InfoWindow infoWindow = null;
 
     private boolean infoWindowShown = false;
+    private int topOffsetPixels;
 
     /**
      * Constructor
@@ -124,7 +125,7 @@ public void setInfoWindowAnchor(float u, float v) {
         infoWindowAnchorV = v;
     }
 
-    public void setPosition(LatLng latlng) {
+    public void setPosition(LatLng position) {
         this.position = position;
     }
 
@@ -161,26 +162,26 @@ public void setTitle(String title) {
     }
 
     public void showInfoWindow() {
-
         if (!isVisible() || mapView == null) {
             return;
         }
 
         getInfoWindow().adaptDefaultMarker(this);
-        getInfoWindow().open(this, getPosition(), (int) anchorU, (int) anchorV);
-        getInfoWindow().setBoundMarker(this);
-        infoWindowShown = true;
+        showInfoWindow(getInfoWindow());
     }
 
     public void showInfoWindow(View view){
-
         if (!isVisible() || mapView == null) {
             return;
         }
 
         infoWindow = new InfoWindow(view, mapView);
-        infoWindow.open(this, getPosition(), (int) anchorU, (int) anchorV);
-        infoWindow.setBoundMarker(this);
+        showInfoWindow(infoWindow);
+    }
+
+    private void showInfoWindow(InfoWindow iw) {
+        iw.open(this, getPosition(), (int) anchorU, (int) anchorV + topOffsetPixels);
+        iw.setBoundMarker(this);
         infoWindowShown = true;
     }
 
@@ -222,4 +223,12 @@ public void setVisible(boolean visible) {
 //    void setIcon(BitmapDescriptor icon) {
 //
 //    }
+
+    public int getTopOffsetPixels() {
+        return topOffsetPixels;
+    }
+
+    public void setTopOffsetPixels(int topOffsetPixels) {
+        this.topOffsetPixels = topOffsetPixels;
+    }
 }

android/java/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/annotations/MarkerOptions.java

@@ -66,6 +66,10 @@ public String getTitle() {
         return ((Marker)annotation).getTitle();
     }
 
+    public String getSprite() {
+        return ((Marker)annotation).getSprite();
+    }
+
     public MarkerOptions infoWindowAnchor(float u, float v) {
         ((Marker)annotation).setInfoWindowAnchor(u, v);
         return this;
@@ -84,7 +88,7 @@ public boolean isVisible() {
     }
 
     public MarkerOptions position(LatLng position) {
-        ((Marker)annotation).position = position;
+        ((Marker)annotation).setPosition(position);
         return this;
     }
 
@@ -100,7 +104,7 @@ public MarkerOptions snippet(String snippet) {
 
     public MarkerOptions sprite(@Nullable String sprite) {
         if (!TextUtils.isEmpty(sprite)) {
-            ((Marker)annotation).sprite = sprite;
+            ((Marker)annotation).setSprite(sprite);
         }
         return this;
     }

android/java/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/views/MapView.java

@@ -35,6 +35,7 @@
 import android.support.v7.app.AlertDialog;
 import android.text.TextUtils;
 import android.util.AttributeSet;
+import android.util.Log;
 import android.view.GestureDetector;
 import android.view.Gravity;
 import android.view.InputDevice;
@@ -311,17 +312,22 @@ public final class MapView extends FrameLayout {
     public static final int REGION_DID_CHANGE_ANIMATED = 4;
 
     /**
-     * Currently not implemented.
+     * This event is triggered when the map is about to start loading a new map style.
+     * <p/>
+     * This event is followed by {@link MapView#DID_FINISH_LOADING_MAP} or
+     * {@link MapView#DID_FAIL_LOADING_MAP}.
      */
     public static final int WILL_START_LOADING_MAP = 5;
 
     /**
-     * Currently not implemented.
+     * This event is triggered when the map has successfully loaded a new map style.
      */
     public static final int DID_FINISH_LOADING_MAP = 6;
 
     /**
      * Currently not implemented.
+     * <p/>
+     * This event is triggered when the map has failed to load a new map style.
      */
     public static final int DID_FAIL_LOADING_MAP = 7;
 
@@ -799,9 +805,13 @@ public void onCreate(@Nullable Bundle savedInstanceState) {
         addOnMapChangedListener(new OnMapChangedListener() {
             @Override
             public void onMapChanged(@MapChange int change) {
-                if (change == REGION_WILL_CHANGE || change == REGION_WILL_CHANGE_ANIMATED) {
+                if ((change == REGION_WILL_CHANGE) || (change == REGION_WILL_CHANGE_ANIMATED)) {
                     deselectMarker();
                 }
+
+                if (change == DID_FINISH_LOADING_MAP) {
+                    adjustTopOffsetPixels();
+                }
             }
         });
     }
@@ -1620,6 +1630,8 @@ public Marker addMarker(@NonNull MarkerOptions markerOptions) {
             //marker.setSprite(DEFAULT_SPRITE);
         }
 
+        marker.setTopOffsetPixels(getTopOffsetPixelsForAnnotationSymbol(marker.getSprite()));
+
         long id = mNativeMapView.addMarker(marker);
         marker.setId(id);        // the annotation needs to know its id
         marker.setMapView(this); // the annotation needs to know which map view it is in
@@ -1657,7 +1669,10 @@ public List<Marker> addMarkers(@NonNull List<MarkerOptions> markerOptionsList) {
                 marker.setSprite("default_marker");
                 //marker.setSprite(DEFAULT_SPRITE);
             }
-            markers.add(markerOptions.getMarker());
+
+            marker.setTopOffsetPixels(getTopOffsetPixelsForAnnotationSymbol(marker.getSprite()));
+
+            markers.add(marker);
         }
 
         long[] ids = mNativeMapView.addMarkers(markers);
@@ -1869,8 +1884,14 @@ private List<Marker> getMarkersInBounds(@NonNull BoundingBox bbox) {
      * @param symbolName Annotation Symbol
      * @return Top Offset in pixels
      */
-    public double getTopOffsetPixelsForAnnotationSymbol(@NonNull String symbolName) {
-        return mNativeMapView.getTopOffsetPixelsForAnnotationSymbol(symbolName);
+    private int getTopOffsetPixelsForAnnotationSymbol(String symbolName) {
+        // This method will dead lock if map paused. Causes a freeze if you add a marker in an
+        // activity's onCreate()
+        if (mNativeMapView.isPaused()) {
+            return 0;
+        }
+
+        return (int) (mNativeMapView.getTopOffsetPixelsForAnnotationSymbol(symbolName) * mScreenDensity);
     }
 
     /**
@@ -1887,15 +1908,6 @@ public double getMetersPerPixelAtLatitude(@FloatRange(from = -180, to = 180) dou
         return mNativeMapView.getMetersPerPixelAtLatitude(latitude, getZoomLevel());
     }
 
-    /**
-     * Get ScreenDensity of device
-     *
-     * @return Screen Density ratio
-     */
-    public float getScreenDensity() {
-        return mScreenDensity;
-    }
-
     private void selectMarker(Marker marker) {
         if (marker == null) {
             return;
@@ -1914,20 +1926,26 @@ private void selectMarker(Marker marker) {
             handledDefaultClick = mOnMarkerClickListener.onMarkerClick(marker);
         }
 
+        if (!handledDefaultClick) {
+            // default behaviour
+            // Can't do this as InfoWindow will get hidden
+            //setCenterCoordinate(marker.getPosition(), true);
+            showInfoWindow(marker);
+        }
+
+        mSelectedMarker = marker;
+    }
+
+    private void showInfoWindow(Marker marker) {
         if (mInfoWindowAdapter != null) {
             // end developer is using a custom InfoWindowAdapter
             View content = mInfoWindowAdapter.getInfoWindow(marker);
             if (content != null) {
                 marker.showInfoWindow(content);
             }
-        } else if (!handledDefaultClick) {
-            // default behaviour
-            // Can't do this as InfoWindow will get hidden
-            //setCenterCoordinate(marker.getPosition(), true);
+        } else {
             marker.showInfoWindow();
         }
-
-        mSelectedMarker = marker;
     }
 
     private void deselectMarker() {
@@ -1939,6 +1957,23 @@ private void deselectMarker() {
         }
     }
 
+    private void adjustTopOffsetPixels() {
+        for (Annotation annotation : mAnnotations) {
+            if (annotation instanceof Marker) {
+                Marker marker = (Marker) annotation;
+                marker.setTopOffsetPixels(
+                        getTopOffsetPixelsForAnnotationSymbol(marker.getSprite()));
+            }
+        }
+
+        if (mSelectedMarker != null) {
+            if (mSelectedMarker.isInfoWindowShown()) {
+                mSelectedMarker.hideInfoWindow();
+                showInfoWindow(mSelectedMarker);
+            }
+        }
+    }
+
     //
     // Rendering
     //

android/java/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/views/NativeMapView.java

@@ -88,6 +88,10 @@ public void pause() {
         nativePause(mNativeMapViewPtr);
     }
 
+    public boolean isPaused() {
+        return nativeIsPaused(mNativeMapViewPtr);
+    }
+
     public void resume() {
         nativeResume(mNativeMapViewPtr);
     }
@@ -471,6 +475,8 @@ private native void nativeCreateSurface(long nativeMapViewPtr,
 
     private native void nativePause(long nativeMapViewPtr);
 
+    private native boolean nativeIsPaused(long nativeMapViewPtr);
+
     private native void nativeResume(long nativeMapViewPtr);
 
     private native void nativeUpdate(long nativeMapViewPtr);

src/mbgl/map/map.cpp

@@ -74,6 +74,10 @@ void Map::renderSync() {
     } else if (renderState != RenderState::fully) {
         renderState = RenderState::fully;
         view.notifyMapChange(MapChangeDidFinishRenderingMapFullyRendered);
+        if (data->loading) {
+            data->loading = false;
+            view.notifyMapChange(MapChangeDidFinishLoadingMap);
+        }
     }
 
     // Triggers an asynchronous update, that eventually triggers a view
@@ -93,10 +97,12 @@ void Map::update(Update flags) {
 #pragma mark - Style
 
 void Map::setStyleURL(const std::string &url) {
+    view.notifyMapChange(MapChangeWillStartLoadingMap);
     context->invoke(&MapContext::setStyleURL, url);
 }
 
 void Map::setStyleJSON(const std::string& json, const std::string& base) {
+    view.notifyMapChange(MapChangeWillStartLoadingMap);
     context->invoke(&MapContext::setStyleJSON, json, base);
 }
 

src/mbgl/map/map_context.cpp

@@ -92,10 +92,6 @@ void MapContext::triggerUpdate(const TransformState& state, const Update flags)
 }
 
 void MapContext::setStyleURL(const std::string& url) {
-    if (styleURL == url) {
-        return;
-    }
-
     FileSource* fs = util::ThreadContext::getFileSource();
 
     if (styleRequest) {
@@ -120,15 +116,12 @@ void MapContext::setStyleURL(const std::string& url) {
             loadStyleJSON(res.data, base);
         } else {
             Log::Error(Event::Setup, "loading style failed: %s", res.message.c_str());
+            data.loading = false;
         }
     });
 }
 
 void MapContext::setStyleJSON(const std::string& json, const std::string& base) {
-    if (styleJSON == json) {
-        return;
-    }
-
     styleURL.clear();
     styleJSON = json;
 
@@ -146,6 +139,10 @@ void MapContext::loadStyleJSON(const std::string& json, const std::string& base)
     // force style cascade, causing all pending transitions to complete.
     style->cascade();
 
+    // set loading here so we don't get a false loaded event as soon as map is
+    // created but before a style is loaded
+    data.loading = true;
+
     updateFlags |= Update::DefaultTransition | Update::Classes | Update::Zoom | Update::Annotations;
     asyncUpdate->send();
 }

src/mbgl/map/map_data.hpp

@@ -146,6 +146,7 @@ class MapData {
     bool paused = false;
     std::mutex mutexPause;
     std::condition_variable condPause;
+    bool loading = false;
 };
 
 }