Skip to content
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

Build triangle tree from geo_shape indexed fields #50012

Merged
merged 3 commits into from
Dec 12, 2019
Merged

Build triangle tree from geo_shape indexed fields #50012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b65b79f
Build triangle tree from geo_shape indexed fields
iverase Dec 10, 2019
74c7a88
Fix extent logic.
iverase Dec 10, 2019
2ff09bb
more friendly values
iverase Dec 10, 2019
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
104 changes: 101 additions & 3 deletions server/src/main/java/org/elasticsearch/common/geo/CentroidCalculator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,19 @@

package org.elasticsearch.common.geo;

import org.elasticsearch.geometry.Circle;
import org.elasticsearch.geometry.Geometry;
import org.elasticsearch.geometry.GeometryCollection;
import org.elasticsearch.geometry.GeometryVisitor;
import org.elasticsearch.geometry.Line;
import org.elasticsearch.geometry.LinearRing;
import org.elasticsearch.geometry.MultiLine;
import org.elasticsearch.geometry.MultiPoint;
import org.elasticsearch.geometry.MultiPolygon;
import org.elasticsearch.geometry.Point;
import org.elasticsearch.geometry.Polygon;
import org.elasticsearch.geometry.Rectangle;

/**
* This class keeps a running Kahan-sum of coordinates
* that are to be averaged in {@link TriangleTreeWriter} for use
Expand All @@ -32,12 +45,13 @@ public class CentroidCalculator {
private double sumY;
private int count;

public CentroidCalculator() {
public CentroidCalculator(Geometry geometry) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This class feels a bit brittle since it is used for calculation and then passed as the final result. I wonder if we could pass just X, Y after we are done with calculations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we cannot do that because we need to support arrays of shapes and therefore the calculation of the centroid needs the calculator. This is something it should change in #49887 so I think we should not worry too much. Depending in how we decide to calculate the centroid we would need to change this class accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that if we are to change how the calculation is structured, it is best to do that in the dedicated calculator issue Ignacio mentioned

this.sumX = 0.0;
this.compX = 0.0;
this.sumY = 0.0;
this.compY = 0.0;
this.count = 0;
geometry.visit(new CentroidCalculatorVisitor(this));
}

/**
Expand All @@ -47,7 +61,7 @@ public CentroidCalculator() {
* @param x the x-coordinate of the point
* @param y the y-coordinate of the point
*/
public void addCoordinate(double x, double y) {
private void addCoordinate(double x, double y) {
double correctedX = x - compX;
double newSumX = sumX + correctedX;
compX = (newSumX - sumX) - correctedX;
Expand All @@ -69,7 +83,7 @@ public void addCoordinate(double x, double y) {
*
* @param otherCalculator the other centroid calculator to add from
*/
void addFrom(CentroidCalculator otherCalculator) {
public void addFrom(CentroidCalculator otherCalculator) {
addCoordinate(otherCalculator.sumX, otherCalculator.sumY);
// adjust count
count += otherCalculator.count - 1;
Expand All @@ -88,4 +102,88 @@ public double getX() {
public double getY() {
return sumY / count;
}

private static class CentroidCalculatorVisitor implements GeometryVisitor<Void, IllegalArgumentException> {

private final CentroidCalculator calculator;

private CentroidCalculatorVisitor(CentroidCalculator calculator) {
this.calculator = calculator;
}

@Override
public Void visit(Circle circle) {
calculator.addCoordinate(circle.getX(), circle.getY());
return null;
}

@Override
public Void visit(GeometryCollection<?> collection) {
for (Geometry shape : collection) {
shape.visit(this);
}
return null;
}

@Override
public Void visit(Line line) {

for (int i = 0; i < line.length(); i++) {
calculator.addCoordinate(line.getX(i), line.getY(i));
}
return null;
}

@Override
public Void visit(LinearRing ring) {
for (int i = 0; i < ring.length() - 1; i++) {
calculator.addCoordinate(ring.getX(i), ring.getY(i));
}
return null;
}

@Override
public Void visit(MultiLine multiLine) {
for (Line line : multiLine) {
visit(line);
}
return null;
}

@Override
public Void visit(MultiPoint multiPoint) {
for (Point point : multiPoint) {
visit(point);
}
return null;
}

@Override
public Void visit(MultiPolygon multiPolygon) {
for (Polygon polygon : multiPolygon) {
visit(polygon);
}
return null;
}

@Override
public Void visit(Point point) {
calculator.addCoordinate(point.getX(), point.getY());
return null;
}

@Override
public Void visit(Polygon polygon) {
return visit(polygon.getPolygon());
}

@Override
public Void visit(Rectangle rectangle) {
calculator.addCoordinate(rectangle.getMinX(), rectangle.getMinY());
calculator.addCoordinate(rectangle.getMinX(), rectangle.getMaxY());
calculator.addCoordinate(rectangle.getMaxX(), rectangle.getMinY());
calculator.addCoordinate(rectangle.getMaxX(), rectangle.getMaxY());
return null;
}
}
}
51 changes: 23 additions & 28 deletions server/src/main/java/org/elasticsearch/common/geo/Extent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,12 @@
*/
package org.elasticsearch.common.geo;

import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;

import java.io.IOException;
import java.util.Objects;

/**
* Object representing the extent of a geometry object within a {@link ShapeTreeWriter}.
* Object representing the extent of a geometry object within a {@link TriangleTreeWriter}.
*/
public class Extent implements Writeable {
static final int WRITEABLE_SIZE_IN_BYTES = 24;
public class Extent {

public int top;
public int bottom;
Expand All @@ -47,7 +41,7 @@ public Extent() {
this.posRight = Integer.MIN_VALUE;
}

public Extent(int top, int bottom, int negLeft, int negRight, int posLeft, int posRight) {
private Extent(int top, int bottom, int negLeft, int negRight, int posLeft, int posRight) {
this.top = top;
this.bottom = bottom;
this.negLeft = negLeft;
Expand All @@ -56,10 +50,6 @@ public Extent(int top, int bottom, int negLeft, int negRight, int posLeft, int p
this.posRight = posRight;
}

Extent(StreamInput input) throws IOException {
this(input.readInt(), input.readInt(), input.readInt(), input.readInt(), input.readInt(), input.readInt());
}

public void reset(int top, int bottom, int negLeft, int negRight, int posLeft, int posRight) {
this.top = top;
this.bottom = bottom;
Expand Down Expand Up @@ -88,9 +78,10 @@ public void addRectangle(int bottomLeftX, int bottomLeftY, int topRightX, int to
this.negRight = Math.max(this.negRight, topRightX);
} else if (bottomLeftX < 0) {
this.negLeft = Math.min(this.negLeft, bottomLeftX);
this.negRight = Math.max(this.negRight, bottomLeftX);
this.posLeft = Math.min(this.posLeft, topRightX);
this.posRight = Math.max(this.posRight, topRightX);
// this signal the extent cannot be wrapped around the dateline
this.negRight = 0;
this.posLeft = 0;
} else {
this.posLeft = Math.min(this.posLeft, bottomLeftX);
this.posRight = Math.max(this.posRight, topRightX);
Expand Down Expand Up @@ -131,8 +122,11 @@ static Extent fromPoints(int bottomLeftX, int bottomLeftY, int topRightX, int to
negLeft = bottomLeftX;
negRight = topRightX;
} else if (bottomLeftX < 0) {
negLeft = negRight = bottomLeftX;
posLeft = posRight = topRightX;
negLeft = bottomLeftX;
posRight = topRightX;
// this signal the extent cannot be wrapped around the dateline
negRight = 0;
posLeft = 0;
} else {
posLeft = bottomLeftX;
posRight = topRightX;
Expand Down Expand Up @@ -168,17 +162,6 @@ public int maxX() {
return Math.max(negRight, posRight);
}


@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeInt(top);
out.writeInt(bottom);
out.writeInt(negLeft);
out.writeInt(negRight);
out.writeInt(posLeft);
out.writeInt(posRight);
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
Expand All @@ -196,4 +179,16 @@ public boolean equals(Object o) {
public int hashCode() {
return Objects.hash(top, bottom, negLeft, negRight, posLeft, posRight);
}

@Override
public String toString() {
StringBuilder builder = new StringBuilder("[");
builder.append("top = " + top + ", ");
builder.append("bottom = " + bottom + ", ");
builder.append("negLeft = " + negLeft + ", ");
builder.append("negRight = " + negRight + ", ");
builder.append("posLeft = " + posLeft + ", ");
builder.append("posRight = " + posRight + "]");
return builder.toString();
}
}
34 changes: 0 additions & 34 deletions server/src/main/java/org/elasticsearch/common/geo/ShapeTreeWriter.java
View file
Open in desktop

This file was deleted.

Loading