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.

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

Bugs/5953 bug bbox #100

Merged
merged 3 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,17 @@

import au.org.aodn.ogcapi.server.core.model.enumeration.CQLCrsType;
import au.org.aodn.ogcapi.server.core.model.enumeration.CQLFieldsInterface;
import au.org.aodn.ogcapi.server.core.util.BboxUtils;
import au.org.aodn.ogcapi.server.core.util.GeometryUtils;
import co.elastic.clients.elasticsearch._types.TopLeftBottomRightGeoBounds;
import co.elastic.clients.elasticsearch._types.query_dsl.BoolQuery;
import org.geotools.filter.spatial.BBOXImpl;
import org.geotools.geometry.jts.JTS;
import org.geotools.geometry.jts.JTSFactoryFinder;
import org.geotools.geometry.jts.ReferencedEnvelope;
import org.geotools.geometry.jts.ReferencedEnvelope3D;
import org.geotools.referencing.CRS;
import org.locationtech.jts.geom.Geometry;
import org.locationtech.jts.geom.GeometryFactory;
import org.locationtech.jts.geom.MultiLineString;
import org.opengis.filter.FilterVisitor;
import org.opengis.filter.MultiValuedFilter;
import org.opengis.filter.expression.Expression;
Expand Down Expand Up @@ -94,8 +93,8 @@ public BBoxImpl(
this.bounds = new ReferencedEnvelope(minx, maxx, miny, maxy, crs);

// We need to handle anti-meridian, we normalize the polygon and may split into two polygon to cover
// two area due to crossing -180 <> 180 line
Geometry g = GeometryUtils.normalizePolygon(GeometryUtils.createPolygon(minx, maxx, miny, maxy));
// two area due to crossing -180 <> 180 line, this is because elastic geo_bounding_box assume [180, -180]
Geometry g = BboxUtils.normalizeBbox(minx, maxx, miny, maxy);
this.create2DCQL(e, GeometryUtils.toReferencedEnvelope(g,crs) , matchAction, enumType);

} catch (FactoryException fe) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import au.org.aodn.ogcapi.server.core.model.enumeration.CQLCrsType;
import au.org.aodn.ogcapi.server.core.model.enumeration.CQLFieldsInterface;
import au.org.aodn.ogcapi.server.core.util.GeometryUtils;
import au.org.aodn.ogcapi.server.core.util.BboxUtils;
import lombok.Getter;
import org.geotools.geometry.jts.ReferencedEnvelope;
import org.geotools.referencing.CRS;
Expand Down Expand Up @@ -61,7 +61,7 @@ public BBoxImpl(
crs = null;
}
this.bounds = new ReferencedEnvelope(minx, maxx, miny, maxy, crs);
this.geometry = GeometryUtils.normalizePolygon(GeometryUtils.createPolygon(minx, maxx, miny, maxy));
this.geometry = BboxUtils.normalizeBbox(minx, maxx, miny, maxy);

} catch (FactoryException fe) {
throw new RuntimeException("Failed to setup bbox SRS", fe);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package au.org.aodn.ogcapi.server.core.util;

import org.locationtech.jts.geom.Coordinate;
import org.locationtech.jts.geom.Geometry;
import org.locationtech.jts.geom.LinearRing;
import org.locationtech.jts.geom.Polygon;
import java.util.ArrayList;
import java.util.List;

/**
* Utility for BBox operation only, it assumes the incoming is a bounding box
*/
public class BboxUtils {
/**
* Normalize a bbox by adjusting longitudes to the range [-180, 180], and then split it into two if it
* is cross meridian. We need this because Bbox is assumed to work within this range only.
* @param minx - left
* @param maxx - right
* @param miny - top
* @param maxy - bottom
* @return - Geometry which is bounded [-180, 180]
*/
public static Geometry normalizeBbox(double minx, double maxx, double miny, double maxy) {
// Bounding check, if greater than 360 already cover whole world, so adjust the maxx to something
// meaningful, noted that minx and maxx is not normalized yet so can be anything even beyond 180
if((maxx - minx) >= 360) {
// Value does not matter, it covered whole world which is equal to
minx = -180;
maxx = 180;
}
minx = (minx < -180) ? 180 - Math.abs(180 + minx) : minx;
maxx = (maxx > 180) ? -(maxx - 180) : maxx;
vietnguyengit marked this conversation as resolved.
Show resolved Hide resolved

// Normalized the box, so it is within [-180, 180]
List<Polygon> polygons = new ArrayList<>();
if(maxx >= 0 && maxx <= 180) {
// Normal case
polygons.add((Polygon)createBoxPolygon(minx, maxx, miny, maxy));
}
else {
polygons.add((Polygon)createBoxPolygon(minx, 180, miny, maxy));
polygons.add((Polygon)createBoxPolygon(-180, maxx, miny, maxy));
}
Comment on lines +33 to +40
Copy link
Collaborator

Choose a reason for hiding this comment

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

so you call createBoxPolygon and cast the result to Polygon, can we make return type of createBoxPolygon to Polygon with validating valid polygon?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can

return GeometryUtils.getFactory().createMultiPolygon(polygons.toArray(new Polygon[0]));
}

protected static Geometry createBoxPolygon(double minx, double maxx, double miny, double maxy) {
// If the longitude range crosses the anti-meridian (e.g., maxx > 180)
// Normal case have not cross dateline
Coordinate[] coordinates = new Coordinate[] {
new Coordinate(minx, miny), // Bottom-left corner
new Coordinate(maxx, miny), // Bottom-right corner
new Coordinate(maxx, maxy), // Top-right corner
new Coordinate(minx, maxy), // Top-left corner
new Coordinate(minx, miny) // Closing the loop (bottom-left corner)
};
vietnguyengit marked this conversation as resolved.
Show resolved Hide resolved

// Create a LinearRing for the boundary of the Polygon
LinearRing ring = GeometryUtils.getFactory().createLinearRing(coordinates);

// Create the Polygon using the LinearRing (no holes for simplicity)
return GeometryUtils.getFactory().createPolygon(ring, null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
public class GeometryUtils {

protected static final int PRECISION = 15;

@Getter
protected static GeometryFactory factory = new GeometryFactory(new PrecisionModel(), 4326);

protected static ObjectMapper mapper = new ObjectMapper();
Expand Down Expand Up @@ -223,22 +225,6 @@ public static Geometry normalizePolygon(Geometry polygon) {
return jtsGeometry.getGeom();
}

public static Geometry createPolygon(double minx, double maxx, double miny, double maxy) {
// Define the corners of the bounding box
Coordinate[] coordinates = new Coordinate[] {
new Coordinate(minx, maxy), // Top-Left
new Coordinate(maxx, maxy), // Top-Right
new Coordinate(maxx, miny), // Bottom-Right
new Coordinate(minx, miny), // Bottom-Left
new Coordinate(minx, maxy) // Closing the polygon (back to Top-Left)
};
// Create a LinearRing (boundary of the polygon)
LinearRing shell = factory.createLinearRing(coordinates);

// Create the polygon (no holes)
return factory.createPolygon(shell, null);
}

public static List<ReferencedEnvelope> toReferencedEnvelope(Geometry geometry, CoordinateReferenceSystem crs) {
List<ReferencedEnvelope> result = new ArrayList<>();
if(geometry instanceof MultiPolygon mp) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,6 @@ public void verifyBBoxWorks1() throws CQLException, IOException, FactoryExceptio
*
* @throws CQLException - Will not throw
* @throws IOException - Will not throw
* @throws FactoryException - Will not throw
* @throws TransformException - Will not throw
* @throws ParseException - Will not throw
*/
@Test
public void verifyIntersectionWorks2() throws CQLException, IOException {
Expand All @@ -162,20 +159,17 @@ public void verifyIntersectionWorks2() throws CQLException, IOException {
Geometry g = (Geometry)filter.accept(visitor, geo.get());

Assertions.assertFalse(g.isEmpty());
Assertions.assertTrue(g instanceof Polygon);
Assertions.assertInstanceOf(Polygon.class, g);

Assertions.assertEquals(g.getCentroid().getX(), 168.30090846621448, "getX()");
Assertions.assertEquals(g.getCentroid().getY(), -33.95984804960966, "getY()");
Assertions.assertEquals(168.30090846621448, g.getCentroid().getX(), "getX()");
Assertions.assertEquals(-33.95984804960966, g.getCentroid().getY(), "getY()");
}
/**
* Test almost the same as the verifyIntersectionWorks2, since verifyIntersectionWorks1 create a polygon same as box
* so use Intersect or BBox will result in same result
*
* @throws CQLException - Will not throw
* @throws IOException - Will not throw
* @throws FactoryException - Will not throw
* @throws TransformException - Will not throw
* @throws ParseException - Will not throw
*/
@Test
public void verifyBBoxWorks2() throws CQLException, IOException {
Expand All @@ -199,20 +193,17 @@ public void verifyBBoxWorks2() throws CQLException, IOException {
Geometry g = (Geometry)filter.accept(visitor, geo.get());

Assertions.assertFalse(g.isEmpty());
Assertions.assertTrue(g instanceof Polygon);
Assertions.assertInstanceOf(Polygon.class, g);

Assertions.assertEquals(g.getCentroid().getX(), 168.30090846621448, 0.0000001, "getX()");
Assertions.assertEquals(g.getCentroid().getY(), -33.95984804960966, 0.0000001, "getY()");
Assertions.assertEquals(168.30090846621448, g.getCentroid().getX(), 0.0000001, "getX()");
Assertions.assertEquals(-33.95984804960966, g.getCentroid().getY(), 0.0000001, "getY()");
}
/**
* Similar test as verifyBBoxWorks2, the BBOX not only cross meridian but the sample json have spatial extents
* near equator and span across the whole world
*
* @throws CQLException - Will not throw
* @throws IOException - Will not throw
* @throws FactoryException - Will not throw
* @throws TransformException - Will not throw
* @throws ParseException - Will not throw
*/
@Test
public void verifyBBoxWorks3() throws CQLException, IOException {
Expand All @@ -235,15 +226,15 @@ public void verifyBBoxWorks3() throws CQLException, IOException {
// return value are geo applied the CQL, and in this case only BBOX intersected
Geometry g = (Geometry)filter.accept(visitor, geo.get());

Assertions.assertTrue(g instanceof MultiPolygon);
Assertions.assertInstanceOf(MultiPolygon.class, g);

MultiPolygon mp = (MultiPolygon)g;
Assertions.assertEquals(mp.getNumGeometries(), 2, "Geometries correct");
Assertions.assertEquals(2, mp.getNumGeometries(), "Geometries correct");

Assertions.assertEquals(mp.getGeometryN(0).getCentroid().getX(), -159.53241830835444, 0.0000001, "getX() for 0");
Assertions.assertEquals(mp.getGeometryN(0).getCentroid().getY(), -19.5, 0.0000001, "getY() for 0");
Assertions.assertEquals(-159.53241830835444, mp.getGeometryN(1).getCentroid().getX(), 0.0000001, "getX() for 0");
Assertions.assertEquals(-19.5, mp.getGeometryN(1).getCentroid().getY(), 0.0000001, "getY() for 0");

Assertions.assertEquals(mp.getGeometryN(1).getCentroid().getX(), 151.62121416760516, 0.0000001, "getX() for 1");
Assertions.assertEquals(mp.getGeometryN(1).getCentroid().getY(), -18.000822620336752, 0.0000001, "getY() for 1");
Assertions.assertEquals(151.62121416760516, mp.getGeometryN(0).getCentroid().getX(), 0.0000001, "getX() for 1");
Assertions.assertEquals(-18.000822620336752, mp.getGeometryN(0).getCentroid().getY(), 0.0000001, "getY() for 1");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package au.org.aodn.ogcapi.server.core.util;

import org.junit.jupiter.api.Test;
import org.locationtech.jts.geom.Geometry;
import org.locationtech.jts.geom.Polygon;

import static org.junit.jupiter.api.Assertions.assertEquals;

public class BboxUtilsTest {
/**
* This test have bbox start from -4.6 -> 0 -> 180 -> -180 -> -76.5
*/
@Test
public void verifyNormalizeBbox1() {
Geometry n = BboxUtils.normalizeBbox(-4.614697916267209,314.17320304524225,-76.58044236400484,60.83962132913365);
assertEquals(2, n.getNumGeometries(), "Size correct");

Polygon expect1 = (Polygon)BboxUtils.createBoxPolygon(-4.614697916267209, 180, -76.58044236400484, 60.83962132913365);
assertEquals(expect1, n.getGeometryN(0), "First polygon");

Polygon expect3 = (Polygon)BboxUtils.createBoxPolygon(-180, -134.17320304524225, -76.58044236400484, 60.83962132913365);
assertEquals(expect3, n.getGeometryN(1), "Second polygon");
}
/**
* This test have bbox start from 10 -> 180 -> -180 -> -76.5
*/
@Test
public void verifyNormalizeBbox2() {
Geometry n = BboxUtils.normalizeBbox(10,314.17320304524225,-76.58044236400484,60.83962132913365);
assertEquals(2, n.getNumGeometries(), "Size correct");

Polygon expect1 = (Polygon)BboxUtils.createBoxPolygon(10, 180, -76.58044236400484, 60.83962132913365);
assertEquals(expect1, n.getGeometryN(0), "First polygon");

Polygon expect2 = (Polygon)BboxUtils.createBoxPolygon(-180, -134.17320304524225, -76.58044236400484, 60.83962132913365);
assertEquals(expect2, n.getGeometryN(1), "Second polygon");
}
/**
* The maxx is so big that it cover whole world a few times
*/
@Test
public void verifyNormalizeBboxVeryBigMaxX() {
Geometry n = BboxUtils.normalizeBbox(10,600,-76.58044236400484,60.83962132913365);
assertEquals(1, n.getNumGeometries(), "Size correct");

Polygon expect1 = (Polygon)BboxUtils.createBoxPolygon(-180, 180, -76.58044236400484, 60.83962132913365);
assertEquals(expect1, n.getGeometryN(0), "First polygon min 10");

// Same no matter how small the minx is
n = BboxUtils.normalizeBbox(-200,600,-76.58044236400484,60.83962132913365);
assertEquals(1, n.getNumGeometries(), "Size correct");

expect1 = (Polygon)BboxUtils.createBoxPolygon(-180, 180, -76.58044236400484, 60.83962132913365);
assertEquals(expect1, n.getGeometryN(0), "First polygon min -200");
}
/**
* The minx is so small
*/
@Test
public void verifyNormalizeBboxSmallMinX() {
Geometry n = BboxUtils.normalizeBbox(-250,-60,-76.58044236400484,60.83962132913365);
assertEquals(2, n.getNumGeometries(), "Size correct");

Polygon expect1 = (Polygon)BboxUtils.createBoxPolygon(110, 180, -76.58044236400484, 60.83962132913365);
assertEquals(expect1, n.getGeometryN(0), "First polygon");

Polygon expect2 = (Polygon)BboxUtils.createBoxPolygon(-180, -60, -76.58044236400484, 60.83962132913365);
assertEquals(expect2, n.getGeometryN(1), "Second polygon");
// -351 = 9 after minus 180
n = BboxUtils.normalizeBbox(-351,-60,-76.58044236400484,60.83962132913365);
assertEquals(2, n.getNumGeometries(), "Size correct");

expect1 = (Polygon)BboxUtils.createBoxPolygon(9, 180, -76.58044236400484, 60.83962132913365);
assertEquals(expect1, n.getGeometryN(0), "First polygon");

expect2 = (Polygon)BboxUtils.createBoxPolygon(-180, -60, -76.58044236400484, 60.83962132913365);
assertEquals(expect2, n.getGeometryN(1), "Second polygon");
// -361 = -1 after minus 180, and -2 for maxx will not cover the whole world
n = BboxUtils.normalizeBbox(-361,-2, -76.58044236400484,60.83962132913365);
assertEquals(2, n.getNumGeometries(), "Size correct");

expect1 = (Polygon)BboxUtils.createBoxPolygon(-1, 180, -76.58044236400484, 60.83962132913365);
assertEquals(expect1, n.getGeometryN(0), "First polygon");

expect2 = (Polygon)BboxUtils.createBoxPolygon(-180, -2, -76.58044236400484, 60.83962132913365);
assertEquals(expect2, n.getGeometryN(1), "Second polygon");
// -361 = -1 after minus 180, and -1 for maxx will cover the whole world
n = BboxUtils.normalizeBbox(-361,-1, -76.58044236400484,60.83962132913365);
assertEquals(1, n.getNumGeometries(), "Size correct");

expect1 = (Polygon)BboxUtils.createBoxPolygon(-180, 180, -76.58044236400484, 60.83962132913365);
assertEquals(expect1, n.getGeometryN(0), "First polygon");
}
/**
* Very small minx so it cover whole world
*/
@Test
public void verifyNormalizeBboxVerySmallMinX() {
Geometry n = BboxUtils.normalizeBbox(-650,-60,-76.58044236400484,60.83962132913365);
assertEquals(1, n.getNumGeometries(), "Size correct");

Polygon expect1 = (Polygon)BboxUtils.createBoxPolygon(-180, 180, -76.58044236400484, 60.83962132913365);
assertEquals(expect1, n.getGeometryN(0), "First polygon min 10");
}
}
Loading