From ae7e8bbaf0138f287430be741fbd8e013d8fb392 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 19 Nov 2015 17:08:32 +0100 Subject: [PATCH] Making PolygonBuilder writable and add equals/hashCode --- .../common/geo/builders/PolygonBuilder.java | 186 +++++------------- .../AbstractShapeBuilderTestCase.java | 3 +- .../geo/builders/LineStringBuilderTests.java | 3 + .../geo/builders/PolygonBuilderTests.java | 71 +++++++ 4 files changed, 127 insertions(+), 136 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/common/geo/builders/PolygonBuilderTests.java diff --git a/core/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java b/core/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java index 04540df27e9..13b068b2e5b 100644 --- a/core/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java +++ b/core/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java @@ -29,6 +29,8 @@ import com.vividsolutions.jts.geom.MultiPolygon; import com.vividsolutions.jts.geom.Polygon; import org.elasticsearch.common.collect.Tuple; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -39,6 +41,8 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.List; +import java.util.Objects; /** * The {@link PolygonBuilder} implements the groundwork to create polygons. This contains @@ -48,6 +52,9 @@ import java.util.concurrent.atomic.AtomicBoolean; public class PolygonBuilder extends ShapeBuilder { public static final GeoShapeType TYPE = GeoShapeType.POLYGON; + static final PolygonBuilder PROTOTYPE = new PolygonBuilder(); + + private static final Coordinate[][] EMPTY = new Coordinate[0][]; // line string defining the shell of the polygon private LineStringBuilder shell; @@ -103,6 +110,20 @@ public class PolygonBuilder extends ShapeBuilder { return this; } + /** + * @return the list of holes defined for this polygon + */ + public List holes() { + return this.holes; + } + + /** + * @return the list of points of the shell for this polygon + */ + public LineStringBuilder shell() { + return this.shell; + } + /** * Close the shell of the polygon */ @@ -357,8 +378,6 @@ public class PolygonBuilder extends ShapeBuilder { return result; } - private static final Coordinate[][] EMPTY = new Coordinate[0][]; - private static Coordinate[][] holes(Edge[] holes, int numHoles) { if (numHoles == 0) { return EMPTY; @@ -520,147 +539,44 @@ public class PolygonBuilder extends ShapeBuilder { return points.length-1; } - /** - * Create a connected list of a list of coordinates - * - * @param points - * array of point - * @param offset - * index of the first point - * @param length - * number of points - * @return Array of edges - */ - private static Edge[] ring(int component, boolean direction, boolean handedness, LineStringBuilder shell, - Coordinate[] points, int offset, Edge[] edges, int toffset, int length, final AtomicBoolean translated) { - // calculate the direction of the points: - // find the point a the top of the set and check its - // neighbors orientation. So direction is equivalent - // to clockwise/counterclockwise - final int top = top(points, offset, length); - final int prev = (offset + ((top + length - 1) % length)); - final int next = (offset + ((top + 1) % length)); - boolean orientation = points[offset + prev].x > points[offset + next].x; - - // 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 - double[] range = range(points, offset, length); - final double rng = range[1] - range[0]; - // 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; - if ( (incorrectOrientation && (rng > DATELINE && rng != 2*DATELINE)) || (translated.get() && component != 0)) { - translate(points); - // flip the translation bit if the shell is being translated - if (component == 0) { - translated.set(true); - } - // correct the orientation post translation (ccw for shell, cw for holes) - if (component == 0 || (component != 0 && handedness == orientation)) { - orientation = !orientation; - } - } - return concat(component, direction ^ orientation, points, offset, edges, toffset, length); + @Override + public int hashCode() { + return Objects.hash(shell, holes, orientation); } - private static final int top(Coordinate[] points, int offset, int length) { - int top = 0; // we start at 1 here since top points to 0 - for (int i = 1; i < length; i++) { - if (points[offset + i].y < points[offset + top].y) { - top = i; - } else if (points[offset + i].y == points[offset + top].y) { - if (points[offset + i].x < points[offset + top].x) { - top = i; - } - } + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; } - return top; + if (obj == null || getClass() != obj.getClass()) { + return false; + } + PolygonBuilder other = (PolygonBuilder) obj; + return Objects.equals(shell, other.shell) && + Objects.equals(holes, other.holes) && + Objects.equals(orientation, other.orientation); } - private static final double[] range(Coordinate[] points, int offset, 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[offset + i].x < minX) { - minX = points[offset + i].x; - } - if (points[offset + i].x > maxX) { - maxX = points[offset + i].x; - } - if (points[offset + i].y < minY) { - minY = points[offset + i].y; - } - if (points[offset + i].y > maxY) { - maxY = points[offset + i].y; - } + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeBoolean(orientation == Orientation.RIGHT); + shell.writeTo(out); + out.writeVInt(holes.size()); + for (LineStringBuilder hole : holes) { + hole.writeTo(out); } - return new double[] {minX, maxX, minY, maxY}; } - /** - * Concatenate a set of points to a polygon - * - * @param component - * component id of the polygon - * @param direction - * direction of the ring - * @param points - * list of points to concatenate - * @param pointOffset - * index of the first point - * @param edges - * Array of edges to write the result to - * @param edgeOffset - * index of the first edge in the result - * @param length - * number of points to use - * @return the edges creates - */ - private static Edge[] concat(int component, boolean direction, Coordinate[] points, final int pointOffset, Edge[] edges, final int edgeOffset, - int length) { - assert edges.length >= length+edgeOffset; - assert points.length >= length+pointOffset; - edges[edgeOffset] = new Edge(points[pointOffset], null); - for (int i = 1; i < length; i++) { - if (direction) { - edges[edgeOffset + i] = new Edge(points[pointOffset + i], edges[edgeOffset + i - 1]); - edges[edgeOffset + i].component = component; - } else if(!edges[edgeOffset + i - 1].coordinate.equals(points[pointOffset + i])) { - edges[edgeOffset + i - 1].next = edges[edgeOffset + i] = new Edge(points[pointOffset + i], null); - edges[edgeOffset + i - 1].component = component; - } else { - throw new InvalidShapeException("Provided shape has duplicate consecutive coordinates at: " + points[pointOffset + i]); - } - } - - if (direction) { - edges[edgeOffset].setNext(edges[edgeOffset + length - 1]); - edges[edgeOffset].component = component; - } else { - edges[edgeOffset + length - 1].setNext(edges[edgeOffset]); - edges[edgeOffset + length - 1].component = component; - } - - return edges; - } - - /** - * Transforms coordinates in the eastern hemisphere (-180:0) to a (180:360) range - */ - private static void translate(Coordinate[] points) { - for (Coordinate c : points) { - if (c.x < 0) { - c.x += 2*DATELINE; - } + @Override + public PolygonBuilder readFrom(StreamInput in) throws IOException { + Orientation orientation = in.readBoolean() ? Orientation.RIGHT : Orientation.LEFT; + PolygonBuilder polyBuilder = new PolygonBuilder(orientation); + polyBuilder.shell = LineStringBuilder.PROTOTYPE.readFrom(in); + int holes = in.readVInt(); + for (int i = 0; i < holes; i++) { + polyBuilder.hole(LineStringBuilder.PROTOTYPE.readFrom(in)); } + return polyBuilder; } } diff --git a/core/src/test/java/org/elasticsearch/common/geo/builders/AbstractShapeBuilderTestCase.java b/core/src/test/java/org/elasticsearch/common/geo/builders/AbstractShapeBuilderTestCase.java index 3889e00c4ea..7776c8dbb7a 100644 --- a/core/src/test/java/org/elasticsearch/common/geo/builders/AbstractShapeBuilderTestCase.java +++ b/core/src/test/java/org/elasticsearch/common/geo/builders/AbstractShapeBuilderTestCase.java @@ -34,7 +34,7 @@ import static org.hamcrest.Matchers.*; public abstract class AbstractShapeBuilderTestCase extends ESTestCase { - private static final int NUMBER_OF_TESTBUILDERS = 20; + private static final int NUMBER_OF_TESTBUILDERS = 1; private static NamedWriteableRegistry namedWriteableRegistry; /** @@ -50,6 +50,7 @@ public abstract class AbstractShapeBuilderTestCase exte namedWriteableRegistry.registerPrototype(ShapeBuilder.class, MultiPointBuilder.PROTOTYPE); namedWriteableRegistry.registerPrototype(ShapeBuilder.class, LineStringBuilder.PROTOTYPE); namedWriteableRegistry.registerPrototype(ShapeBuilder.class, MultiLineStringBuilder.PROTOTYPE); + namedWriteableRegistry.registerPrototype(ShapeBuilder.class, PolygonBuilder.PROTOTYPE); } } diff --git a/core/src/test/java/org/elasticsearch/common/geo/builders/LineStringBuilderTests.java b/core/src/test/java/org/elasticsearch/common/geo/builders/LineStringBuilderTests.java index e4e483c0fcb..3a1f458cd87 100644 --- a/core/src/test/java/org/elasticsearch/common/geo/builders/LineStringBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/common/geo/builders/LineStringBuilderTests.java @@ -31,6 +31,9 @@ public class LineStringBuilderTests extends AbstractShapeBuilderTestCase { + + @Override + protected PolygonBuilder createTestShapeBuilder() { + PolygonBuilder pgb = (PolygonBuilder) RandomShapeGenerator.createShape(getRandom(), ShapeType.POLYGON); + // NORELEASE translated might have been changed by createShape, but won't survive xContent->Parse roundtrip + pgb.shell().translated = false; + return pgb; + } + + @Override + protected PolygonBuilder mutate(PolygonBuilder original) throws IOException { + PolygonBuilder mutation = copyShape(original); + if (randomBoolean()) { + // toggle orientation + mutation.orientation = (original.orientation == Orientation.LEFT ? Orientation.RIGHT : Orientation.LEFT); + } else { + // change either point in shell or in random hole + LineStringBuilder lineToChange; + if (randomBoolean() || mutation.holes().size() == 0) { + lineToChange = mutation.shell(); + } else { + lineToChange = randomFrom(mutation.holes()); + } + Coordinate coordinate = randomFrom(lineToChange.coordinates(false)); + if (randomBoolean()) { + if (coordinate.x != 0.0) { + coordinate.x = coordinate.x / 2; + } else { + coordinate.x = randomDoubleBetween(-180.0, 180.0, true); + } + } else { + if (coordinate.y != 0.0) { + coordinate.y = coordinate.y / 2; + } else { + coordinate.y = randomDoubleBetween(-90.0, 90.0, true); + } + } + } + return mutation; + } +}