From 7ba869a69ac8551ba532a179716711564295d01d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Paczos?= Date: Tue, 1 May 2018 15:15:30 +0200 Subject: [PATCH 1/2] Revert "[android] - unwrap LatLngBounds for the shortest path when passing to core (#11759)" This reverts commit eb39c80 --- .../mapboxsdk/geometry/LatLngBounds.java | 45 ++++--------------- .../com/mapbox/mapboxsdk/maps/MapboxMap.java | 13 ++---- .../mapboxsdk/geometry/LatLngBoundsTest.java | 22 +-------- 3 files changed, 14 insertions(+), 66 deletions(-) diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/geometry/LatLngBounds.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/geometry/LatLngBounds.java index eec0d566bba..05187cf3333 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/geometry/LatLngBounds.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/geometry/LatLngBounds.java @@ -28,7 +28,7 @@ public class LatLngBounds implements Parcelable { /** * Construct a new LatLngBounds based on its corners, given in NESW * order. - *

+ * * If eastern longitude is smaller than the western one, bounds will include antimeridian. * For example, if the NE point is (10, -170) and the SW point is (-10, 170), then bounds will span over 20 degrees * and cross the antimeridian. @@ -46,11 +46,6 @@ public class LatLngBounds implements Parcelable { this.longitudeWest = westLongitude; } - LatLngBounds(LatLngBounds latLngBounds) { - this(latLngBounds.latitudeNorth, latLngBounds.longitudeEast, - latLngBounds.latitudeSouth, latLngBounds.longitudeWest); - } - /** * Returns the world bounds. * @@ -80,6 +75,7 @@ public LatLng getCenter() { if (longCenter >= GeometryConstants.MAX_LONGITUDE) { longCenter = this.longitudeEast - halfSpan; } + return new LatLng(latCenter, longCenter); } return new LatLng(latCenter, longCenter); @@ -192,6 +188,7 @@ public double getLongitudeSpan() { return GeometryConstants.LONGITUDE_SPAN - longSpan; } + static double getLongitudeSpan(final double longEast, final double longWest) { double longSpan = Math.abs(longEast - longWest); if (longEast >= longWest) { @@ -202,25 +199,6 @@ static double getLongitudeSpan(final double longEast, final double longWest) { return GeometryConstants.LONGITUDE_SPAN - longSpan; } - /** - * If bounds cross the antimeridian, unwrap west longitude for the shortest path. - * - * @return unwrapped bounds - */ - public LatLngBounds unwrapBounds() { - double unwrapedLonWest = longitudeWest; - if (longitudeEast < longitudeWest) { - if (longitudeWest > 0 && longitudeEast < 0) { - unwrapedLonWest -= GeometryConstants.LONGITUDE_SPAN; - } else if (longitudeWest < 0 && longitudeEast > 0) { - unwrapedLonWest += GeometryConstants.LONGITUDE_SPAN; - } - return unwrapped(latitudeNorth, longitudeEast, latitudeSouth, unwrapedLonWest); - } else { - return new LatLngBounds(this); - } - } - /** * Validate if LatLngBounds is empty, determined if absolute distance is * @@ -301,11 +279,12 @@ public LatLng[] toLatLngs() { /** * Constructs a LatLngBounds from doubles representing a LatLng pair. - *

+ * * This values of latNorth and latSouth should be in the range of [-90, 90], * see {@link GeometryConstants#MIN_LATITUDE} and {@link GeometryConstants#MAX_LATITUDE}, * otherwise IllegalArgumentException will be thrown. * latNorth should be greater or equal latSouth, otherwise IllegalArgumentException will be thrown. + * *

* This method doesn't recalculate most east or most west boundaries. * Note that lonEast and lonWest will be wrapped to be in the range of [-180, 180], @@ -339,20 +318,12 @@ public static LatLngBounds from( throw new IllegalArgumentException("LatSouth cannot be less than latNorth"); } - return wrapped(latNorth, lonEast, latSouth, lonWest); - } - - static LatLngBounds wrapped(double latNorth, double lonEast, double latSouth, double lonWest) { lonEast = LatLng.wrap(lonEast, GeometryConstants.MIN_LONGITUDE, GeometryConstants.MAX_LONGITUDE); lonWest = LatLng.wrap(lonWest, GeometryConstants.MIN_LONGITUDE, GeometryConstants.MAX_LONGITUDE); return new LatLngBounds(latNorth, lonEast, latSouth, lonWest); } - static LatLngBounds unwrapped(double latNorth, double lonEast, double latSouth, double lonWest) { - return new LatLngBounds(latNorth, lonEast, latSouth, lonWest); - } - private static double lat_(int z, int y) { double n = Math.PI - 2.0 * Math.PI * y / Math.pow(2.0, z); return Math.toDegrees(Math.atan(0.5 * (Math.exp(n) - Math.exp(-n)))); @@ -364,14 +335,14 @@ private static double lon_(int z, int x) { /** * Constructs a LatLngBounds from a Tile identifier. - *

+ * * Returned bounds will have latitude in the range of Mercator projection. + * @see GeometryConstants#MIN_MERCATOR_LATITUDE + * @see GeometryConstants#MAX_MERCATOR_LATITUDE * * @param z Tile zoom level. * @param x Tile X coordinate. * @param y Tile Y coordinate. - * @see GeometryConstants#MIN_MERCATOR_LATITUDE - * @see GeometryConstants#MAX_MERCATOR_LATITUDE */ public static LatLngBounds from(int z, int x, int y) { return new LatLngBounds(lat_(z, y), lon_(z, x + 1), lat_(z, y + 1), lon_(z, x)); diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java index 745485e2d27..5e36dd0f78e 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java @@ -1540,12 +1540,7 @@ public boolean isAllowConcurrentMultipleOpenInfoWindows() { * @param latLngBounds the bounds to constrain the map with */ public void setLatLngBoundsForCameraTarget(@Nullable LatLngBounds latLngBounds) { - if (latLngBounds == null) { - nativeMapView.setLatLngBounds(latLngBounds); - } else { - //unwrapping the bounds to generate the right convex hull in core - nativeMapView.setLatLngBounds(latLngBounds.unwrapBounds()); - } + nativeMapView.setLatLngBounds(latLngBounds); } /** @@ -1555,9 +1550,9 @@ public void setLatLngBoundsForCameraTarget(@Nullable LatLngBounds latLngBounds) * @param padding the padding to apply to the bounds * @return the camera position that fits the bounds and padding */ - public CameraPosition getCameraForLatLngBounds(@NonNull LatLngBounds latLngBounds, int[] padding) { - // get padded camera position from LatLngBounds, unwrapping the bounds to generate the right convex hull in core - return nativeMapView.getCameraForLatLngBounds(latLngBounds.unwrapBounds(), padding); + public CameraPosition getCameraForLatLngBounds(@Nullable LatLngBounds latLngBounds, int[] padding) { + // get padded camera position from LatLngBounds + return nativeMapView.getCameraForLatLngBounds(latLngBounds, padding); } /** diff --git a/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngBoundsTest.java b/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngBoundsTest.java index c1e497af32d..e072f07fb9a 100644 --- a/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngBoundsTest.java +++ b/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngBoundsTest.java @@ -524,6 +524,7 @@ public void testConstructorChecksEastLongitudeInfinity() { LatLngBounds.from(0, Double.POSITIVE_INFINITY, -20, -20); } + @Test public void testConstructorChecksSouthLatitudeNaN() { exception.expect(IllegalArgumentException.class); @@ -542,7 +543,7 @@ public void testConstructorChecksWesttLongitudeNaN() { public void testConstructorChecksSouthLatitudeGreaterThan90() { exception.expect(IllegalArgumentException.class); exception.expectMessage("latitude must be between -90 and 90"); - LatLngBounds.from(20, 20, 95, 0); + LatLngBounds.from(20, 20,95, 0); } @Test @@ -565,23 +566,4 @@ public void testConstructorCheckLatSouthGreaterLatNorth() { exception.expectMessage("LatSouth cannot be less than latNorth"); LatLngBounds.from(0, 20, 20, 0); } - - @Test - public void testCopyConstructor() { - LatLngBounds bounds = LatLngBounds.from(50, 10, -20, -30); - LatLngBounds copyBounds = new LatLngBounds(bounds); - assertEquals(bounds, copyBounds); - } - - @Test - public void testUnwrapBounds() { - LatLngBounds bounds = LatLngBounds.from(16.5, -172.8, -35.127709, 172.6); - LatLngBounds unwrappedBounds = bounds.unwrapBounds(); - assertEquals(bounds.getCenter().wrap(), unwrappedBounds.getCenter().wrap()); - assertEquals(bounds.getSpan(), unwrappedBounds.getSpan()); - assertTrue(unwrappedBounds.getLonEast() < 0 && unwrappedBounds.getLonWest() < 0); - - LatLngBounds bounds2 = LatLngBounds.from(16.5, -162.8, -35.127709, -177.4); - assertEquals(bounds2, bounds2.unwrapBounds()); - } } From 062bc5cf509d9be98b226d9ba940291a1c4a50bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Paczos?= Date: Tue, 1 May 2018 15:33:33 +0200 Subject: [PATCH 2/2] [android] - unwrap LatLngBounds before creating core object --- .../mapboxsdk/geometry/LatLngBounds.java | 13 +++++-------- .../com/mapbox/mapboxsdk/maps/MapboxMap.java | 2 +- .../mapboxsdk/geometry/LatLngBoundsTest.java | 3 +-- .../android/src/geometry/lat_lng_bounds.cpp | 19 +++++++++++-------- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/geometry/LatLngBounds.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/geometry/LatLngBounds.java index 05187cf3333..90cb56f6051 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/geometry/LatLngBounds.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/geometry/LatLngBounds.java @@ -28,7 +28,7 @@ public class LatLngBounds implements Parcelable { /** * Construct a new LatLngBounds based on its corners, given in NESW * order. - * + *

* If eastern longitude is smaller than the western one, bounds will include antimeridian. * For example, if the NE point is (10, -170) and the SW point is (-10, 170), then bounds will span over 20 degrees * and cross the antimeridian. @@ -75,7 +75,6 @@ public LatLng getCenter() { if (longCenter >= GeometryConstants.MAX_LONGITUDE) { longCenter = this.longitudeEast - halfSpan; } - return new LatLng(latCenter, longCenter); } return new LatLng(latCenter, longCenter); @@ -188,7 +187,6 @@ public double getLongitudeSpan() { return GeometryConstants.LONGITUDE_SPAN - longSpan; } - static double getLongitudeSpan(final double longEast, final double longWest) { double longSpan = Math.abs(longEast - longWest); if (longEast >= longWest) { @@ -279,12 +277,11 @@ public LatLng[] toLatLngs() { /** * Constructs a LatLngBounds from doubles representing a LatLng pair. - * + *

* This values of latNorth and latSouth should be in the range of [-90, 90], * see {@link GeometryConstants#MIN_LATITUDE} and {@link GeometryConstants#MAX_LATITUDE}, * otherwise IllegalArgumentException will be thrown. * latNorth should be greater or equal latSouth, otherwise IllegalArgumentException will be thrown. - * *

* This method doesn't recalculate most east or most west boundaries. * Note that lonEast and lonWest will be wrapped to be in the range of [-180, 180], @@ -335,14 +332,14 @@ private static double lon_(int z, int x) { /** * Constructs a LatLngBounds from a Tile identifier. - * + *

* Returned bounds will have latitude in the range of Mercator projection. - * @see GeometryConstants#MIN_MERCATOR_LATITUDE - * @see GeometryConstants#MAX_MERCATOR_LATITUDE * * @param z Tile zoom level. * @param x Tile X coordinate. * @param y Tile Y coordinate. + * @see GeometryConstants#MIN_MERCATOR_LATITUDE + * @see GeometryConstants#MAX_MERCATOR_LATITUDE */ public static LatLngBounds from(int z, int x, int y) { return new LatLngBounds(lat_(z, y), lon_(z, x + 1), lat_(z, y + 1), lon_(z, x)); diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java index 5e36dd0f78e..cfa71436712 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java @@ -1550,7 +1550,7 @@ public void setLatLngBoundsForCameraTarget(@Nullable LatLngBounds latLngBounds) * @param padding the padding to apply to the bounds * @return the camera position that fits the bounds and padding */ - public CameraPosition getCameraForLatLngBounds(@Nullable LatLngBounds latLngBounds, int[] padding) { + public CameraPosition getCameraForLatLngBounds(@NonNull LatLngBounds latLngBounds, int[] padding) { // get padded camera position from LatLngBounds return nativeMapView.getCameraForLatLngBounds(latLngBounds, padding); } diff --git a/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngBoundsTest.java b/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngBoundsTest.java index e072f07fb9a..c66e4b6fdae 100644 --- a/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngBoundsTest.java +++ b/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngBoundsTest.java @@ -524,7 +524,6 @@ public void testConstructorChecksEastLongitudeInfinity() { LatLngBounds.from(0, Double.POSITIVE_INFINITY, -20, -20); } - @Test public void testConstructorChecksSouthLatitudeNaN() { exception.expect(IllegalArgumentException.class); @@ -543,7 +542,7 @@ public void testConstructorChecksWesttLongitudeNaN() { public void testConstructorChecksSouthLatitudeGreaterThan90() { exception.expect(IllegalArgumentException.class); exception.expectMessage("latitude must be between -90 and 90"); - LatLngBounds.from(20, 20,95, 0); + LatLngBounds.from(20, 20, 95, 0); } @Test diff --git a/platform/android/src/geometry/lat_lng_bounds.cpp b/platform/android/src/geometry/lat_lng_bounds.cpp index ec1a32fed5d..827ee52e95c 100644 --- a/platform/android/src/geometry/lat_lng_bounds.cpp +++ b/platform/android/src/geometry/lat_lng_bounds.cpp @@ -9,14 +9,17 @@ jni::Object LatLngBounds::New(jni::JNIEnv& env, mbgl::LatLngBounds } mbgl::LatLngBounds LatLngBounds::getLatLngBounds(jni::JNIEnv& env, jni::Object bounds) { - static auto swLat = LatLngBounds::javaClass.GetField(env, "latitudeSouth"); - static auto swLon = LatLngBounds::javaClass.GetField(env, "longitudeWest"); - static auto neLat = LatLngBounds::javaClass.GetField(env, "latitudeNorth"); - static auto neLon = LatLngBounds::javaClass.GetField(env, "longitudeEast"); - return mbgl::LatLngBounds::hull( - { bounds.Get(env, swLat), bounds.Get(env, swLon) }, - { bounds.Get(env, neLat), bounds.Get(env, neLon) } - ); + static auto swLatField = LatLngBounds::javaClass.GetField(env, "latitudeSouth"); + static auto swLonField = LatLngBounds::javaClass.GetField(env, "longitudeWest"); + static auto neLatField = LatLngBounds::javaClass.GetField(env, "latitudeNorth"); + static auto neLonField = LatLngBounds::javaClass.GetField(env, "longitudeEast"); + + mbgl::LatLng sw = { bounds.Get(env, swLatField), bounds.Get(env, swLonField) }; + mbgl::LatLng ne = { bounds.Get(env, neLatField), bounds.Get(env, neLonField) }; + + sw.unwrapForShortestPath(ne); + + return mbgl::LatLngBounds::hull(sw, ne); } void LatLngBounds::registerNative(jni::JNIEnv& env) {