From 585920e1ff8d83ef8cdad70dde42ad081e086850 Mon Sep 17 00:00:00 2001 From: osana Date: Thu, 26 Apr 2018 00:55:36 -0400 Subject: [android] added more tests for LatLngBounds.union(), LatLngBounds.intersect() fixed going over antimeridian for both --- .../mapbox/mapboxsdk/geometry/LatLngBounds.java | 226 ++++++++++++++++----- .../mapboxsdk/geometry/LatLngBoundsTest.java | 218 +++++++++++++++++++- 2 files changed, 391 insertions(+), 53 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 90cb56f605..c639e49013 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 @@ -294,6 +294,20 @@ public class LatLngBounds implements Parcelable { @FloatRange(from = GeometryConstants.MIN_LATITUDE, to = GeometryConstants.MAX_LATITUDE) double latSouth, double lonWest) { + checkParams(latNorth, lonEast, latSouth, 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); + } + + private static void checkParams( + @FloatRange(from = GeometryConstants.MIN_LATITUDE, to = GeometryConstants.MAX_LATITUDE) double latNorth, + double lonEast, + @FloatRange(from = GeometryConstants.MIN_LATITUDE, to = GeometryConstants.MAX_LATITUDE) double latSouth, + double lonWest) { + if (Double.isNaN(latNorth) || Double.isNaN(latSouth)) { throw new IllegalArgumentException("latitude must not be NaN"); } @@ -314,11 +328,6 @@ public class LatLngBounds implements Parcelable { if (latNorth < latSouth) { throw new IllegalArgumentException("LatSouth cannot be less than latNorth"); } - - 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); } private static double lat_(int z, int y) { @@ -395,7 +404,7 @@ public class LatLngBounds implements Parcelable { return (longitude <= eastLon) && (longitude >= westLon); } - return (longitude < eastLon) || (longitude > westLon); + return (longitude <= eastLon) || (longitude >= westLon); } /** @@ -426,36 +435,94 @@ public class LatLngBounds implements Parcelable { * @param bounds LatLngBounds to add * @return LatLngBounds */ - public LatLngBounds union(LatLngBounds bounds) { - return union(bounds.getLatNorth(), bounds.getLonEast(), bounds.getLatSouth(), bounds.getLonWest()); + public @NonNull LatLngBounds union(@NonNull LatLngBounds bounds) { + return unionNoParamCheck(bounds.getLatNorth(), bounds.getLonEast(), bounds.getLatSouth(), bounds.getLonWest()); } /** * Returns a new LatLngBounds that stretches to include another LatLngBounds, * given by corner points. + *

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

+ * This method doesn't recalculate most east or most west boundaries. + * Note that eastLon and westLon will be wrapped to be in the range of [-180, 180], + * see {@link GeometryConstants#MIN_LONGITUDE} and {@link GeometryConstants#MAX_LONGITUDE} * - * @param latNorth Northern Latitude - * @param lonEast Eastern Longitude - * @param latSouth Southern Latitude - * @param lonWest Western Longitude - * @return BoundingBox - */ - public LatLngBounds union(final double latNorth, final double lonEast, final double latSouth, final double lonWest) { - double north = (this.latitudeNorth < latNorth) ? latNorth : this.latitudeNorth; - double south = (this.latitudeSouth > latSouth) ? latSouth : this.latitudeSouth; - - if (LatLngSpan.getLongitudeSpan(lonEast, this.longitudeWest) - < LatLngSpan.getLongitudeSpan(this.longitudeEast, lonWest)) { - return new LatLngBounds(north, - lonEast, - south, + * @param northLat Northern Latitude corner point + * @param eastLon Eastern Longitude corner point + * @param southLat Southern Latitude corner point + * @param westLon Western Longitude corner point + * @return LatLngBounds + */ + public @NonNull LatLngBounds union( + @FloatRange(from = GeometryConstants.MIN_LATITUDE, to = GeometryConstants.MAX_LATITUDE)double northLat, + double eastLon, + @FloatRange(from = GeometryConstants.MIN_LATITUDE, to = GeometryConstants.MAX_LATITUDE) double southLat, + double westLon) { + + checkParams(northLat, eastLon, southLat, westLon); + + return unionNoParamCheck(northLat, eastLon, southLat, westLon); + } + + private @NonNull LatLngBounds unionNoParamCheck( + @FloatRange(from = GeometryConstants.MIN_LATITUDE, to = GeometryConstants.MAX_LATITUDE)double northLat, + double eastLon, + @FloatRange(from = GeometryConstants.MIN_LATITUDE, to = GeometryConstants.MAX_LATITUDE) double southLat, + double westLon) { + + northLat = (this.latitudeNorth < northLat) ? northLat : this.latitudeNorth; + southLat = (this.latitudeSouth > southLat) ? southLat : this.latitudeSouth; + + eastLon = LatLng.wrap(eastLon, GeometryConstants.MIN_LONGITUDE, GeometryConstants.MAX_LONGITUDE); + westLon = LatLng.wrap(westLon, GeometryConstants.MIN_LONGITUDE, GeometryConstants.MAX_LONGITUDE); + + // longitudes match + if (this.longitudeEast == eastLon && this.longitudeWest == westLon) { + return new LatLngBounds(northLat, eastLon, southLat, westLon); + } + + boolean eastInThis = containsLongitude(this.longitudeEast, this.longitudeWest, eastLon); + boolean westInThis = containsLongitude(this.longitudeEast, this.longitudeWest, westLon); + boolean thisEastInside = containsLongitude(eastLon, westLon, this.longitudeEast); + boolean thisWestInside = containsLongitude(eastLon, westLon, this.longitudeWest); + + // two intersections on each end - covers entire longitude + if (eastInThis && westInThis && thisEastInside && thisWestInside) { + return new LatLngBounds(northLat, GeometryConstants.MAX_LONGITUDE, southLat, GeometryConstants.MIN_LONGITUDE); + } + + if (eastInThis) { + if (westInThis) { + return new LatLngBounds(northLat, this.longitudeEast, southLat, this.longitudeWest); + } + return new LatLngBounds(northLat, this.longitudeEast, southLat, westLon); + } + + if (thisEastInside) { + if (thisWestInside) { + return new LatLngBounds(northLat, eastLon, southLat, westLon); + } + return new LatLngBounds(northLat, eastLon, southLat, this.longitudeWest); + } + + // bounds do not intersect, find where they will form shortest union + if (LatLngSpan.getLongitudeSpan(eastLon, this.longitudeWest) + < LatLngSpan.getLongitudeSpan(this.longitudeEast, westLon)) { + return new LatLngBounds(northLat, + eastLon, + southLat, this.longitudeWest); } - return new LatLngBounds(north, + return new LatLngBounds(northLat, this.longitudeEast, - south, - lonWest); + southLat, + westLon); } /** @@ -464,32 +531,89 @@ public class LatLngBounds implements Parcelable { * @param box LatLngBounds to intersect with * @return LatLngBounds */ - @Nullable - public LatLngBounds intersect(LatLngBounds box) { - double minLonWest = Math.max(getLonWest(), box.getLonWest()); - double maxLonEast = Math.min(getLonEast(), box.getLonEast()); - if (maxLonEast > minLonWest) { - double minLatSouth = Math.max(getLatSouth(), box.getLatSouth()); - double maxLatNorth = Math.min(getLatNorth(), box.getLatNorth()); - if (maxLatNorth > minLatSouth) { - return new LatLngBounds(maxLatNorth, maxLonEast, minLatSouth, minLonWest); - } - } - return null; + public @Nullable LatLngBounds intersect(@NonNull LatLngBounds box) { + return intersectNoParamCheck(box.getLatNorth(), box.getLonEast(), box.getLatSouth(), box.getLonWest()); } /** * Returns a new LatLngBounds that is the intersection of this with another LatLngBounds + *

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

+ * This method doesn't recalculate most east or most west boundaries. + * Note that eastLon and westLon will be wrapped to be in the range of [-180, 180], + * see {@link GeometryConstants#MIN_LONGITUDE} and {@link GeometryConstants#MAX_LONGITUDE} * - * @param northLatitude Northern Longitude - * @param eastLongitude Eastern Latitude - * @param southLatitude Southern Longitude - * @param westLongitude Western Latitude + * @param northLat Northern Latitude corner point + * @param eastLon Eastern Longitude corner point + * @param southLat Southern Latitude corner point + * @param westLon Western Longitude corner point * @return LatLngBounds */ - public LatLngBounds intersect(double northLatitude, double eastLongitude, double southLatitude, - double westLongitude) { - return intersect(new LatLngBounds(northLatitude, eastLongitude, southLatitude, westLongitude)); + public @Nullable LatLngBounds intersect( + @FloatRange(from = GeometryConstants.MIN_LATITUDE, to = GeometryConstants.MAX_LATITUDE)double northLat, + double eastLon, + @FloatRange(from = GeometryConstants.MIN_LATITUDE, to = GeometryConstants.MAX_LATITUDE) double southLat, + double westLon) { + + checkParams(northLat, eastLon, southLat, westLon); + + return intersectNoParamCheck(northLat, eastLon, southLat, westLon); + } + + private @Nullable LatLngBounds intersectNoParamCheck( + @FloatRange(from = GeometryConstants.MIN_LATITUDE, to = GeometryConstants.MAX_LATITUDE)double northLat, + double eastLon, + @FloatRange(from = GeometryConstants.MIN_LATITUDE, to = GeometryConstants.MAX_LATITUDE) double southLat, + double westLon) { + + double maxsouthLat = Math.max(getLatSouth(), Math.min(GeometryConstants.MAX_LATITUDE, southLat)); + double minnorthLat = Math.min(getLatNorth(), Math.max(GeometryConstants.MIN_LATITUDE, northLat)); + if (minnorthLat < maxsouthLat) { + return null; + } + + eastLon = LatLng.wrap(eastLon, GeometryConstants.MIN_LONGITUDE, GeometryConstants.MAX_LONGITUDE); + westLon = LatLng.wrap(westLon, GeometryConstants.MIN_LONGITUDE, GeometryConstants.MAX_LONGITUDE); + + // longitudes match + if (this.longitudeEast == eastLon && this.longitudeWest == westLon) { + return new LatLngBounds(minnorthLat, eastLon, maxsouthLat, westLon); + } + + boolean eastInThis = containsLongitude(this.longitudeEast, this.longitudeWest, eastLon); + boolean westInThis = containsLongitude(this.longitudeEast, this.longitudeWest, westLon); + boolean thisEastInside = containsLongitude(eastLon, westLon, this.longitudeEast); + boolean thisWestInside = containsLongitude(eastLon, westLon, this.longitudeWest); + + // two intersections : find the one that has longest span + if (eastInThis && westInThis && thisEastInside && thisWestInside) { + + if (getLongitudeSpan(eastLon, this.longitudeWest) > getLongitudeSpan(this.longitudeEast, westLon)) { + return new LatLngBounds(minnorthLat, eastLon, maxsouthLat, this.longitudeWest); + } + + return new LatLngBounds(minnorthLat, this.longitudeEast, maxsouthLat, westLon); + } + + if (eastInThis) { + if (westInThis) { + return new LatLngBounds(minnorthLat, eastLon, maxsouthLat, westLon); + } + return new LatLngBounds(minnorthLat, eastLon, maxsouthLat, this.longitudeWest); + } + + if (thisEastInside) { + if (thisWestInside) { + return new LatLngBounds(minnorthLat, this.longitudeEast, maxsouthLat, this.longitudeWest); + } + return new LatLngBounds(minnorthLat, this.longitudeEast, maxsouthLat, westLon); + } + + return null; } /** @@ -518,7 +642,7 @@ public class LatLngBounds implements Parcelable { return (int) ((latitudeNorth + 90) + ((latitudeSouth + 90) * 1000) + ((longitudeEast + 180) * 1000000) - + ((longitudeEast + 180) * 1000000000)); + + ((longitudeWest + 180) * 1000000000)); } /** @@ -546,11 +670,11 @@ public class LatLngBounds implements Parcelable { } private static LatLngBounds readFromParcel(final Parcel in) { - final double lonNorth = in.readDouble(); - final double latEast = in.readDouble(); - final double lonSouth = in.readDouble(); - final double latWest = in.readDouble(); - return new LatLngBounds(lonNorth, latEast, lonSouth, latWest); + final double northLat = in.readDouble(); + final double eastLon = in.readDouble(); + final double southLat = in.readDouble(); + final double westLon = in.readDouble(); + return new LatLngBounds(northLat, eastLon, southLat, westLon); } /** 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 c66e4b6fda..789a1b2b37 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 @@ -356,6 +356,58 @@ public class LatLngBoundsTest { assertNull(latLngBounds.intersect(this.latLngBounds)); } + @Test + public void intersectNorthCheck() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("latitude must be between -90 and 90"); + LatLngBounds intersectLatLngBounds = + LatLngBounds.from(10, 10, 0, 0) + .intersect(200, 200, 0, 0); + } + + @Test + public void intersectSouthCheck() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("latitude must be between -90 and 90"); + LatLngBounds intersectLatLngBounds = + LatLngBounds.from(0, 0, -10, -10) + .intersect(0, 0, -200, -200); + } + + @Test + public void intersectSouthLessThanNorthCheck() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("LatSouth cannot be less than latNorth"); + + LatLngBounds intersectLatLngBounds = + LatLngBounds.from(10, 10, 0, 0) + .intersect(0, 200, 20, 0); + } + + + @Test + public void intersectEastWrapCheck() { + + LatLngBounds latLngBounds1 = LatLngBounds.from(10, -150, 0, 0); + LatLngBounds latLngBounds2 = LatLngBounds.from(90, 200, 0, 0); + + LatLngBounds intersectLatLngBounds = LatLngBounds.from(10, -160, 0, 0); + + assertEquals(latLngBounds1.intersect(latLngBounds2), intersectLatLngBounds); + assertEquals(latLngBounds2.intersect(latLngBounds1), intersectLatLngBounds); + } + + @Test + public void intersectWestWrapCheck() { + LatLngBounds latLngBounds1 = LatLngBounds.from(0, 0, -10, 150); + LatLngBounds latLngBounds2 = LatLngBounds.from(0, 0, -90, -200); + + LatLngBounds intersectLatLngBounds = LatLngBounds.from(0, 0, -10, 160); + + assertEquals(latLngBounds1.intersect(latLngBounds2), intersectLatLngBounds); + assertEquals(latLngBounds2.intersect(latLngBounds1), intersectLatLngBounds); + } + @Test public void innerUnion() { LatLngBounds latLngBounds = new LatLngBounds.Builder() @@ -391,14 +443,176 @@ public class LatLngBoundsTest { .include(new LatLng(-10, -160)) .build(); - assertEquals("outer union should match", - latLngBounds1.union(latLngBounds2), + LatLngBounds union1 = latLngBounds1.union(latLngBounds2); + LatLngBounds union2 = latLngBounds2.union(latLngBounds1); + + assertEquals(union1, + new LatLngBounds.Builder() + .include(new LatLng(10, 160)) + .include(new LatLng(-10, -160)) + .build()); + + assertEquals(union1, union2); + } + + @Test + public void unionOverDateLine2() { + LatLngBounds latLngBounds1 = new LatLngBounds.Builder() + .include(new LatLng(10, 170)) + .include(new LatLng(0, 160)) + .build(); + + LatLngBounds latLngBounds2 = new LatLngBounds.Builder() + .include(new LatLng(0, 165)) + .include(new LatLng(-10, -160)) + .build(); + + LatLngBounds union1 = latLngBounds1.union(latLngBounds2); + LatLngBounds union2 = latLngBounds2.union(latLngBounds1); + + assertEquals(union1, + new LatLngBounds.Builder() + .include(new LatLng(10, 160)) + .include(new LatLng(-10, -160)) + .build()); + + assertEquals(union1, union2); + } + + @Test + public void unionOverDateLine3() { + LatLngBounds latLngBounds1 = new LatLngBounds.Builder() + .include(new LatLng(10, -165)) + .include(new LatLng(0, 160)) + .build(); + + LatLngBounds latLngBounds2 = new LatLngBounds.Builder() + .include(new LatLng(0, -170)) + .include(new LatLng(-10, -160)) + .build(); + + LatLngBounds union1 = latLngBounds1.union(latLngBounds2); + LatLngBounds union2 = latLngBounds2.union(latLngBounds1); + + assertEquals(union1, + new LatLngBounds.Builder() + .include(new LatLng(10, 160)) + .include(new LatLng(-10, -160)) + .build()); + + assertEquals(union1, union2); + } + + @Test + public void unionOverDateLine4() { + LatLngBounds latLngBounds1 = new LatLngBounds.Builder() + .include(new LatLng(10, -160)) + .include(new LatLng(0, 160)) + .build(); + + LatLngBounds latLngBounds2 = new LatLngBounds.Builder() + .include(new LatLng(0, -170)) + .include(new LatLng(-10, -175)) + .build(); + + LatLngBounds union1 = latLngBounds1.union(latLngBounds2); + LatLngBounds union2 = latLngBounds2.union(latLngBounds1); + + assertEquals(union1, new LatLngBounds.Builder() .include(new LatLng(10, 160)) .include(new LatLng(-10, -160)) .build()); + + assertEquals(union1, union2); } + @Test + public void unionOverDateLine5() { + LatLngBounds latLngBounds1 = new LatLngBounds.Builder() + .include(new LatLng(10, -160)) + .include(new LatLng(0, 160)) + .build(); + + LatLngBounds latLngBounds2 = new LatLngBounds.Builder() + .include(new LatLng(0, 170)) + .include(new LatLng(-10, 175)) + .build(); + + LatLngBounds union1 = latLngBounds1.union(latLngBounds2); + LatLngBounds union2 = latLngBounds2.union(latLngBounds1); + + assertEquals(union1, + new LatLngBounds.Builder() + .include(new LatLng(10, 160)) + .include(new LatLng(-10, -160)) + .build()); + + assertEquals(union1, union2); + } + + @Test + public void unionOverDateLineReturnWorldLonSpan() { + LatLngBounds latLngBounds1 = LatLngBounds.from(10, -160, -10, -10); + LatLngBounds latLngBounds2 = LatLngBounds.from(10, 10, -10, 160); + + LatLngBounds union1 = latLngBounds1.union(latLngBounds2); + LatLngBounds union2 = latLngBounds2.union(latLngBounds1); + + assertEquals(union1, union2); + assertEquals(union1, LatLngBounds.from(10, 180, -10, -180)); + } + + @Test + public void unionNorthCheck() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("latitude must be between -90 and 90"); + LatLngBounds unionLatLngBounds = + LatLngBounds.from(10, 10, 0, 0) + .union(200, 200, 0, 0); + } + + @Test + public void unionSouthCheck() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("latitude must be between -90 and 90"); + LatLngBounds unionLatLngBounds = + LatLngBounds.from(0, 0, -10, -10) + .union(0, 0, -200, -200); + } + + @Test + public void unionSouthLessThanNorthCheck() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("LatSouth cannot be less than latNorth"); + + LatLngBounds unionLatLngBounds = + LatLngBounds.from(10, 10, 0, 0) + .union(0, 200, 20, 0); + } + + + @Test + public void unionEastWrapCheck() { + + LatLngBounds latLngBounds1 = LatLngBounds.from(10, 10, 0, 0); + LatLngBounds latLngBounds2 = LatLngBounds.from(90, 200, 0, 0); + LatLngBounds unionLatLngBounds = LatLngBounds.from(90, -160, 0, 0); + + assertEquals(latLngBounds1.union(latLngBounds2), unionLatLngBounds); + assertEquals(latLngBounds2.union(latLngBounds1), unionLatLngBounds); + } + + @Test + public void unionWestWrapCheck() { + LatLngBounds latLngBounds1 = LatLngBounds.from(0, 0, -10, -10); + LatLngBounds latLngBounds2 = LatLngBounds.from(0, 0, -90, -200); + + LatLngBounds unionLatLngBounds = LatLngBounds.from(0, 0, -90, 160); + + assertEquals(latLngBounds1.union(latLngBounds2), unionLatLngBounds); + assertEquals(latLngBounds2.union(latLngBounds1), unionLatLngBounds); + } @Test public void northWest() { -- cgit v1.2.1