Skip to content

Commit bdaad02

Browse files
authored
CentroidCalculator does not return negative summation weights (#135176) (#135190)
Small polygons with holes can compute negative weights due to rounding errors and break serialization for those geometries.
1 parent c7df675 commit bdaad02

File tree

3 files changed

+45
-1
lines changed

3 files changed

+45
-1
lines changed

docs/changelog/135176.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 135176
2+
summary: '`CentroidCalculator` does not return negative summation weights'
3+
area: Geo
4+
type: bug
5+
issues:
6+
- 131861

server/src/main/java/org/elasticsearch/lucene/spatial/CentroidCalculator.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,14 @@ public double getY() {
6363
* @return the sum of all the weighted coordinates summed in the calculator
6464
*/
6565
public double sumWeight() {
66-
return visitor.compSumWeight.value();
66+
double sumWeight = visitor.compSumWeight.value();
67+
if (sumWeight < 0) {
68+
// we can get negative values due to rounding errors in degenerated polygons. The proper fix would be to compute
69+
// the centroid using the tessellation algorithm. For the time being we just return 0.
70+
return 0d;
71+
}
72+
73+
return sumWeight;
6774
}
6875

6976
/**

server/src/test/java/org/elasticsearch/lucene/spatial/CentroidCalculatorTests.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import static org.hamcrest.Matchers.closeTo;
3838
import static org.hamcrest.Matchers.equalTo;
3939
import static org.hamcrest.Matchers.greaterThan;
40+
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
4041

4142
public abstract class CentroidCalculatorTests extends ESTestCase {
4243
private static final double DELTA = 0.000000001;
@@ -398,6 +399,36 @@ public void testAddDifferentDimensionalType() {
398399
}
399400
}
400401

402+
public void testDegeneratedPolygon() {
403+
LinearRing outer = new LinearRing(
404+
new double[] {
405+
144.9738739160867,
406+
144.97387390626216,
407+
144.97387394861903,
408+
144.97387410671985,
409+
144.97387431344814,
410+
144.97387422754971,
411+
144.97387403679602,
412+
144.9738739160867 },
413+
new double[] {
414+
-37.812764320048906,
415+
-37.81276430546101,
416+
-37.81276433502755,
417+
-37.81276449365169,
418+
-37.81276470177617,
419+
-37.8127646444251,
420+
-37.812764516780305,
421+
-37.812764320048906 }
422+
);
423+
LinearRing inner = new LinearRing(
424+
new double[] { 144.9738739160867, 144.97387396264085, 144.97387400134224, 144.97387405619358, 144.9738739160867 },
425+
new double[] { -37.812764320048906, -37.81276437348886, -37.81276441404389, -37.81276447205519, -37.812764320048906 }
426+
);
427+
CentroidCalculator polygonCalculator = new CentroidCalculator();
428+
polygonCalculator.add(new Polygon(outer, List.of(inner)));
429+
assertThat(polygonCalculator.sumWeight(), greaterThanOrEqualTo(0d));
430+
}
431+
401432
private Matcher<CentroidCalculator> matchesCentroid(Point point, double weight) {
402433
return new CentroidMatcher(point.getX(), point.getY(), weight, 1.0);
403434
}

0 commit comments

Comments
 (0)