Merge pull request #1310 from urschrei/push-wsywnvrunyxo

Fix orientation of boolops geometries
georust · Jan 31, 2025 · 6881359 · 6881359
2 parents ef55eab + c086485
commit 6881359
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 8 deletions.
diff --git a/geo/CHANGES.md b/geo/CHANGES.md
@@ -24,6 +24,8 @@
   println!("{}", errors[0]);
   ```
   - BREAKING: update proj to 0.28.0
+- Polygons returned by Boolean Ops are now oriented correctly (ccw shell, cw inner rings)
+  - <https://github.com/georust/geo/pull/1310>
 
 ## 0.29.3 - 2024.12.03
 

diff --git a/geo/src/algorithm/bool_ops/i_overlay_integration.rs b/geo/src/algorithm/bool_ops/i_overlay_integration.rs
@@ -46,8 +46,16 @@ pub(super) mod convert {
     }
 
     pub fn polygon_from_shape<T: BoolOpsNum>(shape: Vec<Vec<BoolOpsCoord<T>>>) -> Polygon<T> {
-        let mut rings = shape.into_iter().map(|p| line_string_from_path(p));
+        let mut rings = shape.into_iter().map(|path| {
+            // From i_overlay: > Note: Outer boundary paths have a clockwise order, and holes have a counterclockwise order.
+            // Which is the opposite convention we use.
+            let mut line_string = line_string_from_path(path);
+            line_string.close();
+            line_string.0.reverse();
+            line_string
+        });
         let exterior = rings.next().unwrap_or(LineString::new(vec![]));
+
         Polygon::new(exterior, rings.collect())
     }
 
@@ -82,9 +90,61 @@ pub(super) mod convert {
 
 #[cfg(test)]
 mod tests {
+    use geo_types::polygon;
+
     use crate::algorithm::BooleanOps;
     use crate::geometry::{MultiPolygon, Polygon};
-    use crate::wkt;
+    use crate::winding_order::WindingOrder;
+    use crate::{wkt, Winding};
+
+    #[test]
+    // see https://github.com/georust/geo/issues/1309
+    fn test_winding_order() {
+        let poly1 = polygon!((x: 0.0, y: 0.0), (x: 1.0, y: 0.0), (x: 1.0, y: 1.0));
+        assert!(matches!(
+            poly1.exterior().winding_order(),
+            Some(WindingOrder::CounterClockwise)
+        ));
+
+        {
+            let union = poly1.union(&polygon!());
+            assert_eq!(union.0.len(), 1);
+
+            let union = &union.0[0];
+            assert!(matches!(
+                union.exterior().winding_order(),
+                Some(WindingOrder::CounterClockwise)
+            ));
+        }
+        {
+            let intersection = poly1.intersection(&poly1);
+            assert_eq!(intersection.0.len(), 1);
+
+            let intersection = &intersection.0[0];
+            assert!(matches!(
+                intersection.exterior().winding_order(),
+                Some(WindingOrder::CounterClockwise)
+            ));
+        }
+
+        let poly2 = polygon!((x: 0.0, y: 0.0), (x: 1.0, y: 1.0), (x: 0.0, y: 1.0));
+        assert!(matches!(
+            poly2.exterior().winding_order(),
+            Some(WindingOrder::CounterClockwise)
+        ));
+
+        {
+            let union = poly1.union(&poly2);
+            assert_eq!(union.0.len(), 1);
+
+            let union = &union.0[0];
+            assert!(union.interiors().is_empty());
+            assert!(matches!(
+                union.exterior().winding_order(),
+                Some(WindingOrder::CounterClockwise)
+            ));
+        }
+    }
 
     #[test]
     fn two_empty_polygons() {
@@ -96,7 +156,7 @@ mod tests {
 
     #[test]
     fn one_empty_polygon() {
-        let p1: Polygon = wkt!(POLYGON((0. 0., 0. 1., 1. 1., 1. 0., 0. 0.)));
+        let p1: Polygon = wkt!(POLYGON((0.0 0.0,1.0 0.0,1.0 1.0,0.0 1.0,0.0 0.0)));
         let p2: Polygon = wkt!(POLYGON EMPTY);
         assert_eq!(&p1.union(&p2), &MultiPolygon(vec![p1.clone()]));
         assert_eq!(&p1.intersection(&p2), &wkt!(MULTIPOLYGON EMPTY));

diff --git a/geo/src/algorithm/bool_ops/mod.rs b/geo/src/algorithm/bool_ops/mod.rs
@@ -158,9 +158,9 @@ pub enum OpType {
 ///
 /// let expected_output = wkt!(MULTIPOLYGON(
 ///     // left and right piece have been combined
-///     ((0. 0., 0. 4., 8. 4., 8. 0.,  0. 0.)),
+///     ((0.0 0.0, 8.0 0.0, 8.0 4.0, 0.0 4.0, 0.0 0.0)),
 ///     // separate piece remains separate
-///     ((14. 10., 14. 14., 18. 14.,18. 10., 14. 10.))
+///     ((14.0 10.0, 18.0 10.0, 18.0 14.0, 14.0 14.0, 14.0 10.0))
 /// ));
 /// assert_eq!(actual_output, expected_output);
 /// ```

diff --git a/geo/src/algorithm/bool_ops/tests.rs b/geo/src/algorithm/bool_ops/tests.rs
@@ -352,15 +352,15 @@ mod gh_issues {
         let expected_c = wkt!(MULTIPOLYGON(
             ((
                 -22.058823 - 3.623188,
-                -19.458324 - 3.623188,
                 -19.064932 - 6.57369,
+                -19.458324 - 3.623188,
                 -22.058823 - 3.623188
             )),
             ((
                 -17.60358 - 8.013862,
-                -14.705883 - 7.649791,
-                -14.705883 - 7.6497912,
                 -17.60358 - 8.013863,
+                -14.705883 - 7.6497912,
+                -14.705883 - 7.649791,
                 -17.60358 - 8.013862
             ))
         ));