Fix android coordinates

android/src/main/java/com/rnmapbox/rnmbx/components/mapview/NativeMapViewModule.kt

@@ -12,6 +12,7 @@ import com.rnmapbox.rnmbx.utils.ExpressionParser
 import com.rnmapbox.rnmbx.utils.ViewRefTag
 import com.rnmapbox.rnmbx.utils.ViewTagResolver
 import com.rnmapbox.rnmbx.utils.extensions.toCoordinate
+import com.rnmapbox.rnmbx.utils.extensions.toScreenBox
 import com.rnmapbox.rnmbx.utils.extensions.toScreenCoordinate
 import com.rnmapbox.rnmbx.utils.extensions.toValueHashMap
 
@@ -116,7 +117,7 @@ class NativeMapViewModule(context: ReactApplicationContext, val viewTagResolver:
             val layerIds = ConvertUtils.toStringList(withLayerIDs)
 
             it.queryRenderedFeaturesAtPoint(
-                ConvertUtils.toPointF(atPoint),
+                atPoint.toScreenCoordinate(),
                 ExpressionParser.from(withFilter),
                 if (layerIds.size == 0) null else layerIds,
                 createCommandResponse(promise)
@@ -135,7 +136,7 @@ class NativeMapViewModule(context: ReactApplicationContext, val viewTagResolver:
             val layerIds = ConvertUtils.toStringList(withLayerIDs)
 
             it.queryRenderedFeaturesInRect(
-                ConvertUtils.toRectF(withBBox),
+                if (withBBox.size() == 0) null else withBBox.toScreenBox(),
                 ExpressionParser.from(withFilter),
                 if (layerIds.size == 0) null else layerIds,
                 createCommandResponse(promise)

android/src/main/java/com/rnmapbox/rnmbx/components/mapview/RNMBXMapView.kt

@@ -2,11 +2,8 @@ package com.rnmapbox.rnmbx.components.mapview
 
 import android.content.Context
 import android.graphics.BitmapFactory
-import android.graphics.PointF
-import android.graphics.RectF
 import android.os.Handler
 import android.os.Looper
-import android.util.Log
 import android.view.Gravity
 import android.view.View
 import android.view.View.OnLayoutChangeListener
@@ -701,24 +698,26 @@ open class RNMBXMapView(private val mContext: Context, var mManager: RNMBXMapVie
                 return true
             }
         }
-        val screenPoint = mMap?.pixelForCoordinate(point)
+        val screenPointPx = mMap?.pixelForCoordinate(point)
         val touchableSources = allTouchableSources
         val hits = HashMap<String?, List<Feature?>?>()
-        if (screenPoint != null) {
-            handleTapInSources(LinkedList(touchableSources), screenPoint, hits, ArrayList(), object : HandleTap {
+        if (screenPointPx != null) {
+            handleTapInSources(LinkedList(touchableSources), screenPointPx, hits, ArrayList(), object : HandleTap {
                 override fun run(hitTouchableSources: List<RNMBXSource<*>?>?, hits: Map<String?, List<Feature?>?>) {
+                    /** Android Mapbox SDK returns screen coordinates in physical pixels, while JS expects density-independent pixels. */
+                    val screenPointDp = toDp(screenPointPx)
                     if (hits.size > 0) {
                         val source = getTouchableSourceWithHighestZIndex(hitTouchableSources as List<RNMBXSource<*>>?)
                         if (source != null && source.hasPressListener() && source.iD != null && source.iD in hits) {
                             source.onPress(RNMBXSource.OnPressEvent(
                                     hits[source.iD] as List<Feature>,
                                     GeoJSONUtils.toLatLng(point),
-                                    PointF(screenPoint.x.toFloat(), screenPoint.y.toFloat())
+                                    screenPointDp
                             ))
                             return
                         }
                     }
-                    val event = MapClickEvent(_this, LatLng(point), screenPoint)
+                    val event = MapClickEvent(_this, LatLng(point), screenPointDp)
                     mManager.handleEvent(event)
                 }
 
@@ -732,9 +731,11 @@ open class RNMBXMapView(private val mContext: Context, var mManager: RNMBXMapVie
         if (pointAnnotations.getAndClearAnnotationDragged()) {
             return true
         }
-        val screenPoint = mMap?.pixelForCoordinate(point)
-        if (screenPoint != null) {
-            val event = MapClickEvent(_this, LatLng(point), screenPoint, EventTypes.MAP_LONG_CLICK)
+        val screenPointPx = mMap?.pixelForCoordinate(point)
+        if (screenPointPx != null) {
+            /** Android Mapbox SDK returns screen coordinates in physical pixels, while JS expects density-independent pixels. */
+            val screenPointDp = toDp(screenPointPx)
+            val event = MapClickEvent(_this, LatLng(point), screenPointDp, EventTypes.MAP_LONG_CLICK)
             mManager.handleEvent(event)
         }
 
@@ -908,48 +909,64 @@ open class RNMBXMapView(private val mContext: Context, var mManager: RNMBXMapVie
         }
     }
 
-    private fun getDisplayDensity(): Float {
-        return mContext.resources.displayMetrics.density
+    private fun getDisplayDensity(): Double {
+        return mContext.resources.displayMetrics.density.toDouble()
     }
 
-    fun getCoordinateFromView(pixel: ScreenCoordinate, response: CommandResponse) {
-        val density: Float = getDisplayDensity()
-        val screenCoordinate = ScreenCoordinate(pixel.x * density, pixel.y * density)
+    /** Converts a point from density-independent pixels to physical pixels. */
+    private fun toDp(pointPx: ScreenCoordinate): ScreenCoordinate {
+        val density = getDisplayDensity()
+        return ScreenCoordinate(pointPx.x / density, pointPx.y / density)
+    }
+
+    /** Converts a point from physical pixels to density-independent pixels. */
+    private fun toPx(pointDp: ScreenCoordinate): ScreenCoordinate {
+        val density = getDisplayDensity()
+        return ScreenCoordinate(pointDp.x * density, pointDp.y * density)
+    }
+
+    /** Converts a bounding box from physical pixels to density-independent pixels. */
+    private fun toPx(boxDp: ScreenBox): ScreenBox {
+        val density = getDisplayDensity()
+        return ScreenBox(
+            ScreenCoordinate(boxDp.min.x * density, boxDp.min.y * density),
+            ScreenCoordinate(boxDp.max.x * density, boxDp.max.y * density),
+        )
+    }
 
-        val coordinate = mMap!!.coordinateForPixel(screenCoordinate)
+    fun getCoordinateFromView(pointDp: ScreenCoordinate, response: CommandResponse) {
+        /** Android Mapbox SDK expects screen coordinates expressed as physical pixels, while JS specifies them as density-independent pixels. */
+        val pointPx = toPx(pointDp)
+
+        val coordinate = mMap!!.coordinateForPixel(pointPx)
 
         response.success {
             it.putArray("coordinateFromView", coordinate.toReadableArray())
         }
     }
 
-    fun getPointInView(coordinate: Point, response: CommandResponse) {
-        val point = mMap!!.pixelForCoordinate(coordinate)
-        val density = getDisplayDensity()
-        val pointInView = PointF((point.x / density).toFloat(), (point.y / density).toFloat())
+    fun getPointInView(coordinates: Point, response: CommandResponse) {
+        /** Android Mapbox SDK returns screen coordinates in physical pixels, while JS expects density-independent pixels. */
+        val pointDp = toDp(mMap!!.pixelForCoordinate(coordinates))
 
         response.success {
             val array: WritableArray = WritableNativeArray()
-            array.pushDouble(pointInView.x.toDouble())
-            array.pushDouble(pointInView.y.toDouble())
+            array.pushDouble(pointDp.x)
+            array.pushDouble(pointDp.y)
             it.putArray("pointInView", array)
         }
     }
 
-    fun queryRenderedFeaturesAtPoint(point: PointF, filter: Expression?, layerIDs: List<String>?, response: CommandResponse) {
+    fun queryRenderedFeaturesAtPoint(pointDp: ScreenCoordinate, filter: Expression?, layerIDs: List<String>?, response: CommandResponse) {
         if (mMap == null) {
             Logger.e("queryRenderedFeaturesAtPoint", "mapbox map is null")
             return
         }
-        // JS sends point values in DIP (see getPointInView which divides by display density),
-        // but Mapbox core expects screen pixel coordinates. Convert back to px here.
-        val density: Float = getDisplayDensity()
-        val screenCoordinate = ScreenCoordinate(
-                (point.x * density).toDouble(),
-                (point.y * density).toDouble()
-        )
-        val queryGeometry = RenderedQueryGeometry(screenCoordinate)
-        val layers = layerIDs?.takeUnless { it.isEmpty() } ?: null;
+
+        /** Android Mapbox SDK expects screen coordinates expressed as physical pixels, while JS specifies them as density-independent pixels. */
+        val pointPx = toPx(pointDp)
+        val queryGeometry = RenderedQueryGeometry(pointPx)
+        val layers = layerIDs?.takeUnless { it.isEmpty() } ?: null
         val queryOptions = RenderedQueryOptions(layers, filter)
         mMap.queryRenderedFeatures(queryGeometry, queryOptions) { features ->
             if (features.isValue) {
@@ -966,14 +983,20 @@ open class RNMBXMapView(private val mContext: Context, var mManager: RNMBXMapVie
         }
     }
 
-    fun queryRenderedFeaturesInRect(rect: RectF?, filter: Expression?, layerIDs: List<String>?, response: CommandResponse) {
+    fun queryRenderedFeaturesInRect(rectDp: ScreenBox?, filter: Expression?, layerIDs: List<String>?, response: CommandResponse) {
         val size = mMap.getMapOptions().size
-        val screenBox = if (rect == null) ScreenBox(ScreenCoordinate(0.0, 0.0), ScreenCoordinate(size?.width!!.toDouble(), size?.height!!.toDouble())) else ScreenBox(
-                ScreenCoordinate(rect.right.toDouble(), rect.bottom.toDouble() ),
-                ScreenCoordinate(rect.left.toDouble(), rect.top.toDouble()),
-        )
+
+        /** Android Mapbox SDK expects screen coordinates expressed as physical pixels, while JS specifies them as density-independent pixels. */
+        val rectPx: ScreenBox =
+            rectDp?.let { toPx(it) }
+                ?: ScreenBox(
+                    ScreenCoordinate(0.0, 0.0),
+                    /** No conversion needed, screen size is already returned in physical pixels. */
+                    ScreenCoordinate(size?.width?.toDouble() ?: 0.0, size?.height?.toDouble() ?: 0.0)
+                )
+
         mMap.queryRenderedFeatures(
-                RenderedQueryGeometry(screenBox),
+                RenderedQueryGeometry(rectPx),
                 RenderedQueryOptions(layerIDs, filter)
         ) { features ->
             if (features.isValue) {

android/src/main/java/com/rnmapbox/rnmbx/components/mapview/RNMBXMapViewManager.kt

@@ -10,25 +10,16 @@ import com.facebook.react.uimanager.LayoutShadowNode
 import com.facebook.react.uimanager.ThemedReactContext
 import com.facebook.react.uimanager.annotations.ReactProp
 import com.rnmapbox.rnmbx.events.constants.EventKeys
-import com.facebook.react.common.MapBuilder
 import com.facebook.react.uimanager.ViewManagerDelegate
 import com.facebook.react.viewmanagers.RNMBXMapViewManagerDelegate
 import com.facebook.react.viewmanagers.RNMBXMapViewManagerInterface
-import com.mapbox.maps.MapInitOptions
 import com.mapbox.maps.extension.style.layers.properties.generated.ProjectionName
 import com.mapbox.maps.plugin.gestures.gestures
-import com.mapbox.maps.plugin.logo.logo
-import com.rnmapbox.rnmbx.events.AndroidCallbackEvent
 import com.rnmapbox.rnmbx.events.constants.eventMapOf
-import com.rnmapbox.rnmbx.utils.ConvertUtils
-import com.rnmapbox.rnmbx.utils.ExpressionParser
 import com.rnmapbox.rnmbx.utils.Logger
 import com.rnmapbox.rnmbx.utils.ViewTagResolver
 import com.rnmapbox.rnmbx.utils.extensions.getAndLogIfNotBoolean
 import com.rnmapbox.rnmbx.utils.extensions.getAndLogIfNotDouble
-import com.rnmapbox.rnmbx.utils.extensions.toCoordinate
-import com.rnmapbox.rnmbx.utils.extensions.toRectF
-import com.rnmapbox.rnmbx.utils.extensions.toScreenCoordinate
 import java.lang.Exception
 import java.util.HashMap
 

android/src/main/java/com/rnmapbox/rnmbx/components/styles/sources/RNMBXSource.kt

@@ -6,22 +6,18 @@ import com.mapbox.maps.extension.style.sources.addSource
 import com.rnmapbox.rnmbx.components.AbstractMapFeature
 import com.rnmapbox.rnmbx.components.mapview.RNMBXMapView
 import com.mapbox.maps.MapboxMap
-import com.rnmapbox.rnmbx.components.styles.sources.AbstractSourceConsumer
 import com.facebook.react.bridge.ReadableMap
-import com.rnmapbox.rnmbx.components.styles.sources.RNMBXSource
-import android.graphics.PointF
 import android.view.View
 import com.facebook.react.common.MapBuilder
 import com.mapbox.geojson.Feature
+import com.mapbox.maps.ScreenCoordinate
 import com.mapbox.maps.Style
 import com.mapbox.maps.extension.style.StyleContract
 import com.mapbox.maps.extension.style.sources.Source
 import com.rnmapbox.rnmbx.components.RemovalReason
-import com.rnmapbox.rnmbx.components.styles.sources.RNMBXSource.OnPressEvent
 import com.rnmapbox.rnmbx.utils.LatLng
 import com.rnmapbox.rnmbx.utils.Logger
 import java.lang.ClassCastException
-import java.util.ArrayList
 import java.util.HashMap
 
 data class FeatureInfo(val feature: AbstractMapFeature?, var added: Boolean) {
@@ -221,7 +217,7 @@ abstract class RNMBXSource<T : Source?>(context: Context?) : AbstractMapFeature(
 
 
     abstract fun makeSource(): T
-    class OnPressEvent(var features: List<Feature>, var latLng: LatLng, var screenPoint: PointF)
+    class OnPressEvent(var features: List<Feature>, var latLng: LatLng, var screenPoint: ScreenCoordinate)
 
     abstract fun onPress(event: OnPressEvent?)
 

android/src/main/java/com/rnmapbox/rnmbx/events/FeatureClickEvent.java

@@ -1,16 +1,15 @@
 package com.rnmapbox.rnmbx.events;
 
-import android.graphics.PointF;
 import android.view.View;
 
 import com.facebook.react.bridge.Arguments;
 import com.facebook.react.bridge.WritableArray;
 import com.facebook.react.bridge.WritableMap;
 import com.mapbox.geojson.Feature;
+import com.mapbox.maps.ScreenCoordinate;
 import com.rnmapbox.rnmbx.components.styles.sources.RNMBXSource;
 import com.rnmapbox.rnmbx.events.constants.EventKeys;
 import com.rnmapbox.rnmbx.events.constants.EventTypes;
-import com.rnmapbox.rnmbx.utils.ConvertUtils;
 import com.rnmapbox.rnmbx.utils.GeoJSONUtils;
 import com.rnmapbox.rnmbx.utils.LatLng;
 
@@ -24,9 +23,9 @@ public class FeatureClickEvent extends AbstractEvent {
     private String mEventKey;
     private List<Feature> mFeatures;
     private LatLng mLatLng;
-    private PointF mPoint;
+    private ScreenCoordinate mPoint;
 
-    public FeatureClickEvent(View view, String eventKey, String eventType, List<Feature> features, LatLng latLng, PointF point) {
+    public FeatureClickEvent(View view, String eventKey, String eventType, List<Feature> features, LatLng latLng, ScreenCoordinate point) {
         super(view, eventType);
         mFeatures = features;
         mEventKey = eventKey;
@@ -55,8 +54,8 @@ public WritableMap getPayload() {
         map.putMap("coordinates", coordinates);
 
         WritableMap point = Arguments.createMap();
-        point.putDouble("x", mPoint.x);
-        point.putDouble("y", mPoint.y);
+        point.putDouble("x", mPoint.getX());
+        point.putDouble("y", mPoint.getY());
         map.putMap("point", point);
 
         return map;

android/src/main/java/com/rnmapbox/rnmbx/utils/ConvertUtils.kt

@@ -1,7 +1,5 @@
 package com.rnmapbox.rnmbx.utils
 
-import android.graphics.PointF
-import android.graphics.RectF
 import android.util.Log
 import com.facebook.react.bridge.Arguments
 import com.facebook.react.bridge.NoSuchKeyException
@@ -172,34 +170,6 @@ object ConvertUtils {
         return list
     }
 
-    fun toPointF(array: ReadableArray?): PointF {
-        val pointF = PointF()
-
-        if (array == null) {
-            return pointF
-        }
-
-        pointF.set(array.getDouble(0).toFloat(), array.getDouble(1).toFloat())
-        return pointF
-    }
-
-    // returns null if array is null
-    fun toRectF(array: ReadableArray?): RectF? {
-        val rectF = RectF()
-
-        if (array == null || array.size() == 0) {
-            return null
-        }
-
-        rectF.set(
-            array.getDouble(3).toFloat(),
-            array.getDouble(0).toFloat(),
-            array.getDouble(1).toFloat(),
-            array.getDouble(2).toFloat()
-        )
-        return rectF
-    }
-
     fun getDouble(key: String, map: ReadableMap, defaultValue: Double): Double {
         var value = defaultValue
 

android/src/main/java/com/rnmapbox/rnmbx/utils/extensions/ReadableArray.kt

@@ -1,17 +1,15 @@
 package com.rnmapbox.rnmbx.utils.extensions
 
-import android.graphics.RectF
 import com.facebook.react.bridge.ReadableArray
 import com.facebook.react.bridge.ReadableType
 import com.google.gson.JsonArray
 import com.google.gson.JsonElement
 import com.mapbox.geojson.Point
+import com.mapbox.maps.ScreenBox
 import com.mapbox.maps.ScreenCoordinate
-import com.rnmapbox.rnmbx.utils.ConvertUtils
 import com.rnmapbox.rnmbx.utils.Logger
-import org.json.JSONArray
-import java.lang.Float.max
-import java.lang.Float.min
+import kotlin.math.max
+import kotlin.math.min
 
 fun ReadableArray.toCoordinate() : Point {
     if (this.size() != 2) {
@@ -25,20 +23,24 @@ fun ReadableArray.toCoordinate() : Point {
 
 fun ReadableArray.toScreenCoordinate() : ScreenCoordinate {
     if (this.size() != 2) {
-        Logger.e("ReadableArray.toCoordinate","Cannot convert $this to point, 2 coordinates are required")
+        Logger.e("ReadableArray.toScreenCoordinate","Cannot convert $this to point, 2 coordinates are required")
     }
     return ScreenCoordinate(getDouble(0), getDouble(1))
 }
 
-fun ReadableArray.toRectF() : RectF? {
-    if (size() != 4) {
-        return null;
+fun ReadableArray.toScreenBox() : ScreenBox {
+    if (this.size() != 4) {
+        Logger.e("ReadableArray.toScreenBox","Cannot convert $this to box, 4 coordinates are required")
     }
-    return RectF(
-        min(getDouble(3).toFloat(), getDouble(1).toFloat()),
-        min(getDouble(0).toFloat(), getDouble(2).toFloat()),
-        max(getDouble(3).toFloat(), getDouble(1).toFloat()),
-        max(getDouble(0).toFloat(), getDouble(2).toFloat())
+
+    val top = getDouble(0)
+    val left = getDouble(1)
+    val bottom = getDouble(2)
+    val right = getDouble(3)
+
+    return ScreenBox(
+        ScreenCoordinate(min(left, right), min(top, bottom)),
+        ScreenCoordinate(max(left, right), max(top, bottom))
     )
 }