Copilot commented on code in PR #2863:
URL: https://github.com/apache/sedona/pull/2863#discussion_r3144299844
##########
common/src/test/java/org/apache/sedona/common/FunctionsTest.java:
##########
@@ -1081,6 +1081,139 @@ public void testS2ToGeom() {
assertTrue(polygons[100].intersects(target));
}
+ @Test
+ public void testS2CoverageContainsInput() throws ParseException {
+ String wkt =
+ "POLYGON ((-102.060778 39.9999603, -102.0535384 40.0119065, -101.98532
40.0122906, "
+ + "-95.30829 40.009008, -95.2456364 39.9564784, -95.1982467
39.9455019, "
+ + "-95.1964657 39.9113444, -95.1460439 39.9142017, -95.1316877
39.8855881, "
+ + "-95.087643 39.8717975, -95.0389987 39.8749063, -95.0146232
39.9088422, "
+ + "-94.9403146 39.906409, -94.9183761 39.8846514, -94.9329504
39.8578468, "
+ + "-94.8824331 39.8409102, -94.8675709 39.8227528, -94.878404
39.787242, "
+ + "-94.9266292 39.7786779, -94.9070076 39.7679231, -94.8631596
39.779774, "
+ + "-94.8497103 39.7604914, -94.8545514 39.7397163, -94.8985678
39.7150641, "
+ + "-94.9584897 39.7331377, -94.9637588 39.6814526, -95.0187723
39.6615712, "
+ + "-95.0456083 39.6252182, -95.0430365 39.5826542, -95.0650104
39.5677387, "
+ + "-95.0985514 39.570063, -95.1012286 39.5462821, -95.0459187
39.5064755, "
+ + "-95.0320969 39.4709074, -94.976457 39.4475392, -94.9362514
39.3964717, "
+ + "-94.8781205 39.3949417, -94.8733156 39.3663291, -94.9014695
39.3495654, "
+ + "-94.8963639 39.3135051, -94.8237994 39.2609956, -94.8194328
39.2178517, "
+ + "-94.7789019 39.214907, -94.7398645 39.1789812, -94.6785238
39.1931279, "
+ + "-94.6533801 39.1816662, -94.6517728 39.1640754, -94.605515
39.1696807, "
+ + "-94.5791896 39.1504025, -94.5983384 39.1134256, -94.6095667
36.9948123, "
+ + "-102.045765 36.9847897, -102.060778 39.9999603))";
+ Geometry input = geomFromWKT(wkt, 0);
+ Long[] cellIds = Functions.s2CellIDs(input, 12);
+ Geometry[] polygons =
+
Functions.s2ToGeom(Arrays.stream(cellIds).mapToLong(Long::longValue).toArray());
+ Geometry coverage = Functions.union(polygons);
+ Geometry uncovered = input.difference(coverage);
+ double uncoveredArea = uncovered.getArea();
+ log.info(
+ "S2 cells: {}, input area: {}, uncovered area: {} ({} %)",
Review Comment:
This test unions all returned cell polygons and then computes
`input.difference(coverage)` unconditionally. For the GH-2857 reproducer this
is ~50k+ cells at level 12, so `union`/`difference` can become a major
CPU/memory cost in unit tests. Consider (a) only computing `difference`/area
when `covers` fails (so the heavy work is on failure only), and/or (b) avoiding
a full geometric union by checking containment against a spatial index /
prepared geometries / sampled boundary points, or using an S2-side containment
check.
##########
common/src/main/java/org/apache/sedona/common/utils/S2Utils.java:
##########
@@ -107,13 +107,189 @@ public static Polygon toJTSPolygon(S2CellId cellId) {
return new GeometryFactory().createPolygon(coords);
}
+ /**
+ * Convert a JTS planar geometry into an S2Region whose lat/lng projection
is guaranteed to
+ * contain the input geometry.
+ *
+ * <p>Why a buffer is needed: Sedona geometries are planar — an edge between
two vertices is a
+ * straight line in (lng, lat) space — but S2 connects the same vertices
with a great-circle arc
+ * on the sphere. The two interpretations agree at the vertices but diverge
along the edges (e.g.
+ * the great-circle arc between two points at the same northern latitude
bulges northward, leaving
+ * the parallel that would form the planar chord). If we hand the JTS
vertices to S2 directly, the
+ * spherical polygon's interior is *smaller* than the planar polygon's
interior along
+ * non-meridional edges, so the S2 covering misses thin slivers of the
original planar polygon
+ * (see GH-2857).
+ *
+ * <p>To compensate, we JTS-buffer the input by an upper bound on the
worst-case great-circle
+ * deviation before converting to S2. A side effect for {@link LineString}
inputs is that the
+ * buffer turns the line into a polygon corridor; downstream callers
therefore see cells in a thin
+ * strip around the line rather than only cells the line geometrically
traverses.
+ */
public static S2Region toS2Region(Geometry geom) throws
IllegalArgumentException {
- if (geom instanceof Polygon) {
- return S2Utils.toS2Polygon((Polygon) geom);
- } else if (geom instanceof LineString) {
- return S2Utils.toS2PolyLine((LineString) geom);
+ if (!(geom instanceof Polygon) && !(geom instanceof LineString)) {
+ throw new IllegalArgumentException(
+ "only object of Polygon, LinearRing, LineString type can be
converted to S2Region");
+ }
+ double eps = arcChordBufferDegrees(geom);
+ Geometry buffered = (eps > 0) ? geom.buffer(eps) : geom;
+ if (buffered instanceof Polygon) {
+ return S2Utils.toS2Polygon((Polygon) buffered);
+ } else if (buffered instanceof LineString) {
+ // Only reachable when eps == 0 (e.g. a single-point degenerate line).
Normal lines
+ // become Polygon corridors after buffer and are handled above.
+ return S2Utils.toS2PolyLine((LineString) buffered);
+ } else if (buffered instanceof MultiPolygon && buffered.getNumGeometries()
> 0) {
+ // JTS buffer of self-touching geometries can collapse to MultiPolygon.
We can only
+ // hand a single S2Region back to callers, so cover the largest piece —
the smaller
+ // pieces are typically tiny artifacts of the buffer operation rather
than real input.
+ Polygon largest = (Polygon) buffered.getGeometryN(0);
+ for (int i = 1; i < buffered.getNumGeometries(); i++) {
+ Polygon p = (Polygon) buffered.getGeometryN(i);
+ if (p.getArea() > largest.getArea()) {
+ largest = p;
+ }
+ }
+ return S2Utils.toS2Polygon(largest);
Review Comment:
If `geom.buffer(eps)` returns a `MultiPolygon`, the current behavior picks
the largest polygon and discards the rest. This can break the stated guarantee
that the resulting S2Region contains the original planar geometry (discarded
components can still contain parts of the buffered/original geometry). Since
`S2Polygon` can represent multiple shells via multiple `S2Loop`s, consider
converting *all* polygons in the MultiPolygon into loops (or unioning the
MultiPolygon before conversion) instead of dropping components.
##########
common/src/test/java/org/apache/sedona/common/FunctionsTest.java:
##########
@@ -1081,6 +1081,139 @@ public void testS2ToGeom() {
assertTrue(polygons[100].intersects(target));
}
+ @Test
+ public void testS2CoverageContainsInput() throws ParseException {
+ String wkt =
+ "POLYGON ((-102.060778 39.9999603, -102.0535384 40.0119065, -101.98532
40.0122906, "
+ + "-95.30829 40.009008, -95.2456364 39.9564784, -95.1982467
39.9455019, "
+ + "-95.1964657 39.9113444, -95.1460439 39.9142017, -95.1316877
39.8855881, "
+ + "-95.087643 39.8717975, -95.0389987 39.8749063, -95.0146232
39.9088422, "
+ + "-94.9403146 39.906409, -94.9183761 39.8846514, -94.9329504
39.8578468, "
+ + "-94.8824331 39.8409102, -94.8675709 39.8227528, -94.878404
39.787242, "
+ + "-94.9266292 39.7786779, -94.9070076 39.7679231, -94.8631596
39.779774, "
+ + "-94.8497103 39.7604914, -94.8545514 39.7397163, -94.8985678
39.7150641, "
+ + "-94.9584897 39.7331377, -94.9637588 39.6814526, -95.0187723
39.6615712, "
+ + "-95.0456083 39.6252182, -95.0430365 39.5826542, -95.0650104
39.5677387, "
+ + "-95.0985514 39.570063, -95.1012286 39.5462821, -95.0459187
39.5064755, "
+ + "-95.0320969 39.4709074, -94.976457 39.4475392, -94.9362514
39.3964717, "
+ + "-94.8781205 39.3949417, -94.8733156 39.3663291, -94.9014695
39.3495654, "
+ + "-94.8963639 39.3135051, -94.8237994 39.2609956, -94.8194328
39.2178517, "
+ + "-94.7789019 39.214907, -94.7398645 39.1789812, -94.6785238
39.1931279, "
+ + "-94.6533801 39.1816662, -94.6517728 39.1640754, -94.605515
39.1696807, "
+ + "-94.5791896 39.1504025, -94.5983384 39.1134256, -94.6095667
36.9948123, "
+ + "-102.045765 36.9847897, -102.060778 39.9999603))";
+ Geometry input = geomFromWKT(wkt, 0);
+ Long[] cellIds = Functions.s2CellIDs(input, 12);
+ Geometry[] polygons =
+
Functions.s2ToGeom(Arrays.stream(cellIds).mapToLong(Long::longValue).toArray());
+ Geometry coverage = Functions.union(polygons);
+ Geometry uncovered = input.difference(coverage);
+ double uncoveredArea = uncovered.getArea();
+ log.info(
+ "S2 cells: {}, input area: {}, uncovered area: {} ({} %)",
+ cellIds.length, input.getArea(), uncoveredArea, (uncoveredArea /
input.getArea()) * 100.0);
+ assertTrue(
+ String.format(
+ "Coverage does not contain input. Missing %.8f deg^2 (%.6f%%)",
+ uncoveredArea, (uncoveredArea / input.getArea()) * 100.0),
+ coverage.covers(input));
+ }
+
+ @Test
+ public void testS2CoverageContainsLineString() throws ParseException {
+ // Long east-west line at mid-latitude. The great-circle arc bulges
poleward of the JTS
+ // chord, so before the buffer fix the cells along the parallel were
missed in the middle.
+ Geometry line = geomFromWKT("LINESTRING (-102 37, -94 37)", 0);
+ Long[] cellIds = Functions.s2CellIDs(line, 12);
+ Geometry[] cells =
+
Functions.s2ToGeom(Arrays.stream(cellIds).mapToLong(Long::longValue).toArray());
+ Geometry coverage = Functions.union(cells);
+ assertTrue(
+ "S2 cell coverage of LineString does not contain the line itself.",
coverage.covers(line));
+ }
+
+ @Test
+ public void testS2CoverageContainsMultiPolygon() throws ParseException {
+ // Two disjoint polygons, both at mid-northern latitude with long
east-west edges.
+ String wkt =
+ "MULTIPOLYGON ("
+ + "((-102 37, -94 37, -94 40, -102 40, -102 37)),"
+ + "((-90 50, -80 50, -80 53, -90 53, -90 50)))";
+ Geometry input = geomFromWKT(wkt, 0);
+ Long[] cellIds = Functions.s2CellIDs(input, 10);
+ Geometry[] cells =
+
Functions.s2ToGeom(Arrays.stream(cellIds).mapToLong(Long::longValue).toArray());
+ Geometry coverage = Functions.union(cells);
+ assertTrue(
+ "S2 cell coverage does not contain every member of the MultiPolygon.",
+ coverage.covers(input));
+ }
+
+ @Test
+ public void testS2CoverageContainsMultiLineString() throws ParseException {
+ // Three disjoint multi-segment lines: northern hemisphere, southern
hemisphere, and a
+ // diagonal climb. Each is decomposed and buffered independently before S2
covering.
+ String wkt =
+ "MULTILINESTRING ("
+ + "(-102 37, -98 37, -94 37),"
+ + "(-90 -42, -85 -42, -80 -42),"
+ + "(-100 50, -95 55, -90 60))";
+ Geometry input = geomFromWKT(wkt, 0);
+ Long[] cellIds = Functions.s2CellIDs(input, 10);
+ Geometry[] cells =
+
Functions.s2ToGeom(Arrays.stream(cellIds).mapToLong(Long::longValue).toArray());
+ Geometry coverage = Functions.union(cells);
+ assertTrue(
+ "S2 cell coverage does not contain every member of the
MultiLineString.",
+ coverage.covers(input));
+ }
+
+ @Test
+ public void testS2CoverageContainsGeometryCollection() throws ParseException
{
+ String wkt =
+ "GEOMETRYCOLLECTION ("
+ + "POLYGON ((-102 37, -94 37, -94 40, -102 40, -102 37)),"
+ + "LINESTRING (10 60, 20 60, 30 60))";
+ Geometry input = geomFromWKT(wkt, 0);
+ Long[] cellIds = Functions.s2CellIDs(input, 10);
+ Geometry[] cells =
+
Functions.s2ToGeom(Arrays.stream(cellIds).mapToLong(Long::longValue).toArray());
+ Geometry coverage = Functions.union(cells);
+ assertTrue(
+ "S2 cell coverage does not contain every member of the
GeometryCollection.",
+ coverage.covers(input));
+ }
+
+ @Test
+ public void testS2CoverageContainsPolygonWithHole() throws ParseException {
+ // Outer ring at mid-latitude with long east-west edges; an inner hole.
Both rings need
+ // their great-circle bulge accounted for so the buffer applies to
interior rings too.
+ String wkt =
+ "POLYGON ("
+ + "(-102 37, -94 37, -94 40, -102 40, -102 37),"
+ + "(-100 38, -96 38, -96 39, -100 39, -100 38))";
+ Geometry input = geomFromWKT(wkt, 0);
+ Long[] cellIds = Functions.s2CellIDs(input, 12);
+ Geometry[] cells =
+
Functions.s2ToGeom(Arrays.stream(cellIds).mapToLong(Long::longValue).toArray());
+ Geometry coverage = Functions.union(cells);
+ assertTrue("S2 cell coverage does not contain a polygon with a hole.",
coverage.covers(input));
+ }
+
+ @Test
+ public void testS2CoverageContainsHighLatitudePolygon() throws
ParseException {
+ // Polygon near 80°N: tan(φ) is large, so the buffer cap (1°) and 89°
latitude clamp need
+ // to keep the arc-chord bound from blowing up.
Review Comment:
This test comment mentions a “buffer cap (1°) and 89° latitude clamp”, but
the current implementation in
`S2Utils.arcChordBufferDegrees/edgeDeviationDegrees` does not apply any
explicit cap or latitude clamp (no occurrences in `S2Utils`). Either implement
those safeguards as described, or update the comment to match the actual
behavior so future readers aren’t misled about how high-latitude cases are
handled.
```suggestion
// Polygon near 80°N with a long east-west edge. This verifies that S2
coverage still
// contains the input at high latitude without assuming any explicit
buffer cap or
// latitude clamp in the underlying arc-chord deviation calculation.
```
##########
common/src/main/java/org/apache/sedona/common/utils/S2Utils.java:
##########
@@ -107,13 +107,189 @@ public static Polygon toJTSPolygon(S2CellId cellId) {
return new GeometryFactory().createPolygon(coords);
}
+ /**
+ * Convert a JTS planar geometry into an S2Region whose lat/lng projection
is guaranteed to
+ * contain the input geometry.
+ *
+ * <p>Why a buffer is needed: Sedona geometries are planar — an edge between
two vertices is a
+ * straight line in (lng, lat) space — but S2 connects the same vertices
with a great-circle arc
+ * on the sphere. The two interpretations agree at the vertices but diverge
along the edges (e.g.
+ * the great-circle arc between two points at the same northern latitude
bulges northward, leaving
+ * the parallel that would form the planar chord). If we hand the JTS
vertices to S2 directly, the
+ * spherical polygon's interior is *smaller* than the planar polygon's
interior along
+ * non-meridional edges, so the S2 covering misses thin slivers of the
original planar polygon
+ * (see GH-2857).
+ *
+ * <p>To compensate, we JTS-buffer the input by an upper bound on the
worst-case great-circle
+ * deviation before converting to S2. A side effect for {@link LineString}
inputs is that the
+ * buffer turns the line into a polygon corridor; downstream callers
therefore see cells in a thin
+ * strip around the line rather than only cells the line geometrically
traverses.
+ */
public static S2Region toS2Region(Geometry geom) throws
IllegalArgumentException {
- if (geom instanceof Polygon) {
- return S2Utils.toS2Polygon((Polygon) geom);
- } else if (geom instanceof LineString) {
- return S2Utils.toS2PolyLine((LineString) geom);
+ if (!(geom instanceof Polygon) && !(geom instanceof LineString)) {
+ throw new IllegalArgumentException(
+ "only object of Polygon, LinearRing, LineString type can be
converted to S2Region");
+ }
+ double eps = arcChordBufferDegrees(geom);
+ Geometry buffered = (eps > 0) ? geom.buffer(eps) : geom;
+ if (buffered instanceof Polygon) {
+ return S2Utils.toS2Polygon((Polygon) buffered);
+ } else if (buffered instanceof LineString) {
+ // Only reachable when eps == 0 (e.g. a single-point degenerate line).
Normal lines
+ // become Polygon corridors after buffer and are handled above.
+ return S2Utils.toS2PolyLine((LineString) buffered);
+ } else if (buffered instanceof MultiPolygon && buffered.getNumGeometries()
> 0) {
+ // JTS buffer of self-touching geometries can collapse to MultiPolygon.
We can only
+ // hand a single S2Region back to callers, so cover the largest piece —
the smaller
+ // pieces are typically tiny artifacts of the buffer operation rather
than real input.
+ Polygon largest = (Polygon) buffered.getGeometryN(0);
+ for (int i = 1; i < buffered.getNumGeometries(); i++) {
+ Polygon p = (Polygon) buffered.getGeometryN(i);
+ if (p.getArea() > largest.getArea()) {
+ largest = p;
+ }
+ }
+ return S2Utils.toS2Polygon(largest);
}
throw new IllegalArgumentException(
"only object of Polygon, LinearRing, LineString type can be converted
to S2Region");
}
+
+ /**
+ * Compute the JTS buffer amount (in degrees) needed so that the spherical
interpretation of the
+ * buffered geometry fully contains the original planar geometry.
+ *
+ * <p>The buffer must be at least as large as the largest great-circle/chord
deviation among the
+ * edges that S2 will eventually see. Polygons and lines need different
bounds because JTS buffer
+ * affects their edges differently:
+ *
+ * <ul>
+ * <li><b>Polygon</b>: each existing edge is offset perpendicularly in
place; corners get
+ * rounded into many short arcs, but no edge is dramatically
lengthened. The buffered
+ * polygon's edges therefore have ~the same length as the originals,
so the original
+ * polygon's per-edge deviation is a tight upper bound on what the
buffered polygon's edges
+ * will exhibit. We use {@link #ringMaxDeviationDegrees}.
+ * <li><b>LineString</b>: buffering produces a corridor whose long
top/bottom edges span the
+ * line's full envelope — far longer than any individual segment when
consecutive segments
+ * are collinear (JTS often simplifies them away). Per-segment
deviation severely
+ * under-bounds the corridor's actual edge deviation. We bound by
virtual edges across the
+ * envelope via {@link #envelopeDeviationDegrees}.
+ * </ul>
+ *
+ * <p>The 1.5× safety multiplier absorbs numerical error and the small
additional deviation the
+ * buffered polygon's own (slightly different) edges introduce on top of the
bound.
+ */
+ private static double arcChordBufferDegrees(Geometry geom) {
+ double maxDev = 0.0;
+ if (geom instanceof Polygon) {
+ Polygon poly = (Polygon) geom;
+ maxDev = Math.max(maxDev,
ringMaxDeviationDegrees(poly.getExteriorRing().getCoordinates()));
+ for (int i = 0; i < poly.getNumInteriorRing(); i++) {
+ maxDev =
+ Math.max(maxDev,
ringMaxDeviationDegrees(poly.getInteriorRingN(i).getCoordinates()));
+ }
+ } else if (geom instanceof LineString) {
+ maxDev = envelopeDeviationDegrees(geom);
+ }
+ return maxDev * 1.5;
+ }
+
+ /**
+ * Conservative deviation upper bound for a geometry, derived from its
bounding envelope rather
+ * than its actual edges.
+ *
+ * <p>Used for {@link LineString} inputs because, after JTS buffer, the
corridor's long edges are
+ * not the line's segments — they connect the line's extreme endpoints (or
close to it). To bound
+ * them we probe three virtual edges across the envelope:
+ *
+ * <ul>
+ * <li>The two diagonals (SW–NE and NW–SE) — diagonal great-circle arcs
deviate more than
+ * east-west arcs of the same Δλ at high latitudes, and a buffered
corridor's long edges can
+ * run in either direction depending on the line's orientation.
+ * <li>The worst-case east-west edge at whichever latitude (top or bottom
of the envelope) has
+ * the larger absolute value — east-west arcs bulge poleward, so the
deviation grows with
+ * |latitude|, and an envelope-spanning east-west arc is what a
horizontal collinear line
+ * would buffer into.
+ * </ul>
+ *
+ * <p>The max across these three bounds the deviation any corridor edge
could plausibly exhibit.
+ * This deliberately over-bounds zigzag lines whose actual corridor edges
are short; the
+ * alternative (per-segment analysis) silently fails on collinear inputs.
+ */
+ private static double envelopeDeviationDegrees(Geometry geom) {
+ Envelope env = geom.getEnvelopeInternal();
+ if (env.isNull()) {
+ return 0.0;
+ }
+ Coordinate sw = new Coordinate(env.getMinX(), env.getMinY());
+ Coordinate se = new Coordinate(env.getMaxX(), env.getMinY());
+ Coordinate ne = new Coordinate(env.getMaxX(), env.getMaxY());
+ Coordinate nw = new Coordinate(env.getMinX(), env.getMaxY());
+ // For the east-west probe, pick whichever latitude band of the envelope
is further from
+ // the equator — that's where same-Δλ great-circle arcs deviate most from
the chord.
+ double signedLat =
+ Math.abs(env.getMinY()) > Math.abs(env.getMaxY()) ? env.getMinY() :
env.getMaxY();
+ Coordinate ewWest = new Coordinate(env.getMinX(), signedLat);
+ Coordinate ewEast = new Coordinate(env.getMaxX(), signedLat);
+ double max = edgeDeviationDegrees(sw, ne);
+ max = Math.max(max, edgeDeviationDegrees(nw, se));
+ max = Math.max(max, edgeDeviationDegrees(ewWest, ewEast));
+ return max;
+ }
+
+ /**
+ * Per-edge deviation bound for a ring/path: walk consecutive vertex pairs
and return the largest
+ * single-edge great-circle/chord deviation. Used for polygon rings
(exterior and interior), where
+ * buffering preserves edge lengths and per-edge analysis is tight.
+ */
+ private static double ringMaxDeviationDegrees(Coordinate[] coords) {
+ double maxDev = 0.0;
+ for (int i = 0; i < coords.length - 1; i++) {
+ double dev = edgeDeviationDegrees(coords[i], coords[i + 1]);
+ if (dev > maxDev) {
+ maxDev = dev;
+ }
+ }
+ return maxDev;
+ }
+
+ /**
+ * Primitive deviation for a single edge: the (lng, lat) distance between
the planar chord
+ * midpoint and the great-circle arc midpoint.
+ *
+ * <p>Why the midpoint: a great-circle arc between two points is symmetric
about its midpoint, and
+ * the (lng, lat) deviation from the chord is maximized there. So the
midpoint deviation equals
+ * the maximum deviation along the edge — there's no need to sample multiple
points.
+ *
+ * <p>The great-circle midpoint is computed by averaging the two endpoint
S2Points (unit vectors
+ * on the sphere) and renormalizing — a standard spherical-midpoint trick.
The chord midpoint is
+ * the plain Euclidean mean of the (lng, lat) coordinates.
+ */
+ private static double edgeDeviationDegrees(Coordinate a, Coordinate b) {
+ S2Point aPt = toS2Point(a);
+ S2Point bPt = toS2Point(b);
+ double mx = aPt.getX() + bPt.getX();
+ double my = aPt.getY() + bPt.getY();
+ double mz = aPt.getZ() + bPt.getZ();
+ double norm = Math.sqrt(mx * mx + my * my + mz * mz);
+ if (norm < 1e-15) {
+ // Antipodal endpoints — the great circle through them is not unique, so
there is no
+ // well-defined midpoint to compare against. Returning 0 effectively
skips this edge;
+ // antipodal inputs aren't realistic for S2 covering anyway.
+ return 0.0;
+ }
+ S2LatLng midSpherical = new S2LatLng(new S2Point(mx / norm, my / norm, mz
/ norm));
+ double midSphericalLat = midSpherical.latDegrees();
+ double midSphericalLng = midSpherical.lngDegrees();
+ double midChordLat = (a.y + b.y) / 2.0;
+ double midChordLng = (a.x + b.x) / 2.0;
+ double dLat = midSphericalLat - midChordLat;
+ double dLng = midSphericalLng - midChordLng;
+ // Wrap longitude difference into [-180, 180]. Without this, an edge
straddling the
+ // antimeridian (e.g. -179° to +179°) would compute dLng ≈ 358° and
produce a bogus
+ // ~360° deviation rather than the small actual deviation.
+ if (dLng > 180.0) dLng -= 360.0;
+ else if (dLng < -180.0) dLng += 360.0;
+ return Math.sqrt(dLat * dLat + dLng * dLng);
Review Comment:
edgeDeviationDegrees computes the chord midpoint longitude as the raw
average `(a.x + b.x) / 2.0`. For edges that cross the antimeridian (e.g., 179°
to -179°), this yields a midpoint near 0° and produces a ~180° ‘deviation’ even
though the true arc/chord deviation is small. The later wrap of `dLng` does not
fix the incorrect midpoint. Compute the chord midpoint using wrapped longitude
deltas (wrap `b.x - a.x` into [-180, 180] and then `a.x + delta/2`, wrapped
back) so antimeridian-spanning geometries don’t get an enormous buffer and
over-covering.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
