Skip to content

Update GeoPolygonFilter to handle polygons crossing the dateline #9339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nknize merged 1 commit into from
Jan 27, 2015
Merged

Update GeoPolygonFilter to handle polygons crossing the dateline #9339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 32 additions & 27 deletions src/main/java/org/elasticsearch/common/geo/GeoPoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,12 @@
package org.elasticsearch.common.geo;


import com.vividsolutions.jts.geom.Coordinate;

/**
*
*/
public final class GeoPoint {

private double lat;
private double lon;
public final class GeoPoint extends Coordinate {

public GeoPoint() {
}
Expand All @@ -41,32 +40,36 @@ public GeoPoint(String value) {
this.resetFromString(value);
}

public GeoPoint(GeoPoint other) {
super(other);
}

public GeoPoint(double lat, double lon) {
this.lat = lat;
this.lon = lon;
this.y = lat;
this.x = lon;
}

public GeoPoint reset(double lat, double lon) {
this.lat = lat;
this.lon = lon;
this.y = lat;
this.x = lon;
return this;
}

public GeoPoint resetLat(double lat) {
this.lat = lat;
this.y = lat;
return this;
}

public GeoPoint resetLon(double lon) {
this.lon = lon;
this.x = lon;
return this;
}

public GeoPoint resetFromString(String value) {
int comma = value.indexOf(',');
if (comma != -1) {
lat = Double.parseDouble(value.substring(0, comma).trim());
lon = Double.parseDouble(value.substring(comma + 1).trim());
this.y = Double.parseDouble(value.substring(0, comma).trim());
this.x = Double.parseDouble(value.substring(comma + 1).trim());
} else {
resetFromGeoHash(value);
}
Expand All @@ -79,38 +82,40 @@ public GeoPoint resetFromGeoHash(String hash) {
}

public final double lat() {
return this.lat;
return this.y;
}

public final double getLat() {
return this.lat;
return this.y;
}

public final double lon() {
return this.lon;
return this.x;
}

public final double getLon() {
return this.lon;
return this.x;
}

public final String geohash() {
return GeoHashUtils.encode(lat, lon);
return GeoHashUtils.encode(y, x);
}

public final String getGeohash() {
return GeoHashUtils.encode(lat, lon);
return GeoHashUtils.encode(y, x);
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;

GeoPoint geoPoint = (GeoPoint) o;

if (Double.compare(geoPoint.lat, lat) != 0) return false;
if (Double.compare(geoPoint.lon, lon) != 0) return false;
if (o == null) return false;
if (o instanceof Coordinate) {
Coordinate c = (Coordinate)o;
return Double.compare(c.x, this.x) == 0
&& Double.compare(c.y, this.y) == 0
&& Double.compare(c.z, this.z) == 0;
}
if (getClass() != o.getClass()) return false;

return true;
}
Expand All @@ -119,15 +124,15 @@ public boolean equals(Object o) {
public int hashCode() {
int result;
long temp;
temp = lat != +0.0d ? Double.doubleToLongBits(lat) : 0L;
temp = y != +0.0d ? Double.doubleToLongBits(y) : 0L;
result = (int) (temp ^ (temp >>> 32));
temp = lon != +0.0d ? Double.doubleToLongBits(lon) : 0L;
temp = x != +0.0d ? Double.doubleToLongBits(x) : 0L;
result = 31 * result + (int) (temp ^ (temp >>> 32));
return result;
}

public String toString() {
return "[" + lat + ", " + lon + "]";
return "[" + y + ", " + x + "]";
Copy link
Member

@rjernst rjernst Jan 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize this was already this way, but the format here (using brackets) looks like GeoJSON, so shouldn't it be [x,y]?

Copy link
Contributor Author

@nknize nknize Jan 21, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To comply with GeoJSON, absolutely. I was a little concerned about changing order for sake of the existing Java API. If ES users are doing anything upstream with that string and we reverse order on them... But I do agree that this should comply with the GeoJSON spec, and so should they.

For consistency we should probably revisit the docs at http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/mapping-geo-point-type.html and the geo_point type in general. The order was originally set to comply with "natural language" lat/lon ordering to make life easy.

Copy link
Contributor Author

@nknize nknize Jan 21, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per our discussion (and the fact that reversing it ripples heavily through the GeoPointFilter and related tests) I'm going to leave this the way it is for now.

}

public static GeoPoint parseFromLatLon(String latLon) {
Expand Down
112 changes: 111 additions & 1 deletion src/main/java/org/elasticsearch/common/geo/GeoUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.common.geo;

import org.apache.commons.lang3.tuple.Pair;
import org.apache.lucene.spatial.prefix.tree.GeohashPrefixTree;
import org.apache.lucene.spatial.prefix.tree.QuadPrefixTree;
import org.apache.lucene.util.SloppyMath;
Expand All @@ -37,7 +38,9 @@ public class GeoUtils {
public static final String LATITUDE = GeoPointFieldMapper.Names.LAT;
public static final String LONGITUDE = GeoPointFieldMapper.Names.LON;
public static final String GEOHASH = GeoPointFieldMapper.Names.GEOHASH;


public static final double DATELINE = 180.0D;

/** Earth ellipsoid major axis defined by WGS 84 in meters */
public static final double EARTH_SEMI_MAJOR_AXIS = 6378137.0; // meters (WGS 84)

Expand Down Expand Up @@ -422,6 +425,113 @@ public static GeoPoint parseGeoPoint(XContentParser parser, GeoPoint point) thro
}
}

public static boolean correctPolyAmbiguity(GeoPoint[] points, boolean handedness) {
return correctPolyAmbiguity(points, handedness, computePolyOrientation(points), 0, points.length, false);
}

public static boolean correctPolyAmbiguity(GeoPoint[] points, boolean handedness, boolean orientation, int component, int length,
boolean shellCorrected) {
// OGC requires shell as ccw (Right-Handedness) and holes as cw (Left-Handedness)
// since GeoJSON doesn't specify (and doesn't need to) GEO core will assume OGC standards
// thus if orientation is computed as cw, the logic will translate points across dateline
// and convert to a right handed system

// compute the bounding box and calculate range
Pair<Pair, Pair> range = GeoUtils.computeBBox(points, length);
final double rng = (Double)range.getLeft().getRight() - (Double)range.getLeft().getLeft();
// translate the points if the following is true
// 1. shell orientation is cw and range is greater than a hemisphere (180 degrees) but not spanning 2 hemispheres
// (translation would result in a collapsed poly)
// 2. the shell of the candidate hole has been translated (to preserve the coordinate system)
boolean incorrectOrientation = component == 0 && handedness != orientation;
boolean translated = ((incorrectOrientation && (rng > DATELINE && rng != 360.0)) || (shellCorrected && component != 0));
if (translated) {
for (GeoPoint c : points) {
if (c.x < 0.0) {
c.x += 360.0;
}
}
}
return translated;
}

public static boolean computePolyOrientation(GeoPoint[] points) {
return computePolyOrientation(points, points.length);
}

public static boolean computePolyOrientation(GeoPoint[] points, int length) {
// calculate the direction of the points:
// find the point at the top of the set and check its
// neighbors orientation. So direction is equivalent
// to clockwise/counterclockwise
final int top = computePolyOrigin(points, length);
final int prev = ((top + length - 1) % length);
final int next = ((top + 1) % length);
return (points[prev].x > points[next].x);
}

private static final int computePolyOrigin(GeoPoint[] points, int length) {
int top = 0;
// we start at 1 here since top points to 0
for (int i = 1; i < length; i++) {
if (points[i].y < points[top].y) {
top = i;
} else if (points[i].y == points[top].y) {
if (points[i].x < points[top].x) {
top = i;
}
}
}
return top;
}

public static final Pair computeBBox(GeoPoint[] points) {
return computeBBox(points, 0);
}

public static final Pair computeBBox(GeoPoint[] points, int length) {
double minX = points[0].x;
double maxX = points[0].x;
double minY = points[0].y;
double maxY = points[0].y;
// compute the bounding coordinates (@todo: cleanup brute force)
for (int i = 1; i < length; ++i) {
if (points[i].x < minX) {
minX = points[i].x;
}
if (points[i].x > maxX) {
maxX = points[i].x;
}
if (points[i].y < minY) {
minY = points[i].y;
}
if (points[i].y > maxY) {
maxY = points[i].y;
}
}
// return a pair of ranges on the X and Y axis, respectively
return Pair.of(Pair.of(minX, maxX), Pair.of(minY, maxY));
}

public static GeoPoint convertToGreatCircle(GeoPoint point) {
return convertToGreatCircle(point.y, point.x);
}

public static GeoPoint convertToGreatCircle(double lat, double lon) {
GeoPoint p = new GeoPoint(lat, lon);
// convert the point to standard lat/lon bounds
normalizePoint(p);

if (p.x < 0.0D) {
p.x += 360.0D;
}

if (p.y < 0.0D) {
p.y +=180.0D;
}
return p;
}

private GeoUtils() {
}
}
30 changes: 15 additions & 15 deletions src/main/java/org/elasticsearch/common/geo/builders/BaseLineStringBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,22 @@
import java.util.ArrayList;
import java.util.Arrays;

import com.spatial4j.core.shape.ShapeCollection;
import org.elasticsearch.common.geo.GeoPoint;
import org.elasticsearch.common.geo.GeoUtils;
import org.elasticsearch.common.xcontent.XContentBuilder;

import com.spatial4j.core.shape.Shape;
import com.vividsolutions.jts.geom.Coordinate;
import com.vividsolutions.jts.geom.Geometry;
import com.vividsolutions.jts.geom.GeometryFactory;
import com.vividsolutions.jts.geom.LineString;

public abstract class BaseLineStringBuilder<E extends BaseLineStringBuilder<E>> extends PointCollection<E> {

protected BaseLineStringBuilder() {
this(new ArrayList<Coordinate>());
this(new ArrayList<GeoPoint>());
}

protected BaseLineStringBuilder(ArrayList<Coordinate> points) {
protected BaseLineStringBuilder(ArrayList<GeoPoint> points) {
super(points);
}

Expand All @@ -49,7 +49,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws

@Override
public Shape build() {
Coordinate[] coordinates = points.toArray(new Coordinate[points.size()]);
GeoPoint[] coordinates = points.toArray(new GeoPoint[points.size()]);
Geometry geometry;
if(wrapdateline) {
ArrayList<LineString> strings = decompose(FACTORY, coordinates, new ArrayList<LineString>());
Expand All @@ -67,9 +67,9 @@ public Shape build() {
return jtsGeometry(geometry);
}

protected static ArrayList<LineString> decompose(GeometryFactory factory, Coordinate[] coordinates, ArrayList<LineString> strings) {
for(Coordinate[] part : decompose(+DATELINE, coordinates)) {
for(Coordinate[] line : decompose(-DATELINE, part)) {
protected static ArrayList<LineString> decompose(GeometryFactory factory, GeoPoint[] coordinates, ArrayList<LineString> strings) {
for(GeoPoint[] part : decompose(+DATELINE, coordinates)) {
for(GeoPoint[] line : decompose(-DATELINE, part)) {
strings.add(factory.createLineString(line));
}
}
Expand All @@ -83,16 +83,16 @@ protected static ArrayList<LineString> decompose(GeometryFactory factory, Coordi
* @param coordinates coordinates forming the linestring
* @return array of linestrings given as coordinate arrays
*/
protected static Coordinate[][] decompose(double dateline, Coordinate[] coordinates) {
protected static GeoPoint[][] decompose(double dateline, GeoPoint[] coordinates) {
int offset = 0;
ArrayList<Coordinate[]> parts = new ArrayList<>();
ArrayList<GeoPoint[]> parts = new ArrayList<>();

double shift = coordinates[0].x > DATELINE ? DATELINE : (coordinates[0].x < -DATELINE ? -DATELINE : 0);

for (int i = 1; i < coordinates.length; i++) {
double t = intersection(coordinates[i-1], coordinates[i], dateline);
if(!Double.isNaN(t)) {
Coordinate[] part;
GeoPoint[] part;
if(t<1) {
part = Arrays.copyOfRange(coordinates, offset, i+1);
part[part.length-1] = Edge.position(coordinates[i-1], coordinates[i], t);
Expand All @@ -111,16 +111,16 @@ protected static Coordinate[][] decompose(double dateline, Coordinate[] coordina
if(offset == 0) {
parts.add(shift(shift, coordinates));
} else if(offset < coordinates.length-1) {
Coordinate[] part = Arrays.copyOfRange(coordinates, offset, coordinates.length);
GeoPoint[] part = Arrays.copyOfRange(coordinates, offset, coordinates.length);
parts.add(shift(shift, part));
}
return parts.toArray(new Coordinate[parts.size()][]);
return parts.toArray(new GeoPoint[parts.size()][]);
}

private static Coordinate[] shift(double shift, Coordinate...coordinates) {
private static GeoPoint[] shift(double shift, GeoPoint...coordinates) {
if(shift != 0) {
for (int j = 0; j < coordinates.length; j++) {
coordinates[j] = new Coordinate(coordinates[j].x - 2 * shift, coordinates[j].y);
coordinates[j] = new GeoPoint(coordinates[j].y, coordinates[j].x - 2 * shift);
}
}
return coordinates;
Expand Down
Loading