iverase commented on a change in pull request #709: LUCENE-8850: Calculate the 
area of a polygon and throw error when values are invalid
URL: https://github.com/apache/lucene-solr/pull/709#discussion_r298453900
 
 

 ##########
 File path: lucene/core/src/java/org/apache/lucene/geo/Polygon.java
 ##########
 @@ -96,34 +99,50 @@ public Polygon(double[] polyLats, double[] polyLons, 
Polygon... holes) {
     this.holes = holes.clone();
 
     // compute bounding box
-    double minLat = polyLats[0];
-    double maxLat = polyLats[0];
-    double minLon = polyLons[0];
-    double maxLon = polyLons[0];
+    double minLat = Double.POSITIVE_INFINITY;
+    double maxLat = Double.NEGATIVE_INFINITY;
+    double minLon = Double.POSITIVE_INFINITY;
+    double maxLon = Double.NEGATIVE_INFINITY;
 
     double windingSum = 0d;
     final int numPts = polyLats.length - 1;
-    for (int i = 1, j = 0; i < numPts; j = i++) {
+    for (int i = 0; i < numPts; i++) {
       minLat = Math.min(polyLats[i], minLat);
       maxLat = Math.max(polyLats[i], maxLat);
       minLon = Math.min(polyLons[i], minLon);
       maxLon = Math.max(polyLons[i], maxLon);
       // compute signed area
-      windingSum += (polyLons[j] - polyLons[numPts])*(polyLats[i] - 
polyLats[numPts])
-          - (polyLats[j] - polyLats[numPts])*(polyLons[i] - polyLons[numPts]);
+      windingSum += polyLons[i] * polyLats[i + 1] - polyLats[i] * polyLons[i + 
1];
+    }
+    if (windingSum == 0) {
+      throw new IllegalArgumentException("Cannot compute the polygon / hole 
orientation.");
     }
     this.minLat = minLat;
     this.maxLat = maxLat;
     this.minLon = minLon;
     this.maxLon = maxLon;
     this.windingOrder = (windingSum < 0) ? GeoUtils.WindingOrder.CCW : 
GeoUtils.WindingOrder.CW;
+    double area = Math.abs(windingSum / 2d);
+    for (Polygon hole : holes) {
+      area -= hole.area();
+    }
+    if (area <= 0) {
+      throw new IllegalArgumentException("Polygon has an invalid area (area = 
" + area + ").");
+    }
+    this.area = area;
+
   }
 
   /** returns the number of vertex points */
   public int numPoints() {
     return polyLats.length;
   }
 
+  /** returns the area of the polygon */
+  public double area() {
 
 Review comment:
   I agree we should not provide this info to the user, +1 not to offer the 
calculation .
   
   The idea behind this change was validation of the polygon. The idea of this 
change came from the fact that I am using this calculation to check that a 
Polygon tessellation is correct and I realise that:
   
   - Currently we compute the signed area to check the orientation of a 
polygon, if the area is 0 the polygon is consider CW, should we fail instead?
   
   - I have seen polygons with one hole where the hole and polygon are the 
same, should we capture that situation and fail?
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to