diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c7b28b98c..d848290f5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -116,7 +116,7 @@ jobs: uses: actions/checkout@v3 - run: cargo check --all-targets --no-default-features # we don't want to test `proj-network` because it only enables the `proj` feature - - run: cargo test --features "use-proj use-serde earcutr multithreading" + - run: cargo test --features "use-proj use-serde earcutr multithreading geos-tests" geo_traits: name: geo-traits diff --git a/geo/Cargo.toml b/geo/Cargo.toml index f4679f83e..10fa17172 100644 --- a/geo/Cargo.toml +++ b/geo/Cargo.toml @@ -18,6 +18,7 @@ use-proj = ["proj"] proj-network = ["use-proj", "proj/network"] use-serde = ["serde", "geo-types/serde"] multithreading = ["i_overlay/allow_multithreading", "geo-types/multithreading"] +geos-tests = ["geos"] [dependencies] earcutr = { version = "0.4.2", optional = true } @@ -28,6 +29,7 @@ geographiclib-rs = { version = "0.2.3", default-features = false } log = "0.4.11" num-traits = "0.2" proj = { version = "0.27.0", optional = true } +geos = { version = "9.1.1", features = ["geo"], optional = true } robust = "1.1.0" rstar = "0.12.0" serde = { version = "1.0", optional = true, features = ["derive"] } @@ -37,14 +39,10 @@ i_overlay = { version = "1.9.0, < 1.10.0", default-features = false } approx = ">= 0.4.0, < 0.6.0" criterion = { version = "0.4", features = ["html_reports"] } geo-test-fixtures = { path = "../geo-test-fixtures" } -# REVIEW: make optional? remove? It's used for some conformance tests which are somewhat redundant with the JTS tests -geos = { version = "9.1.1", features = ["geo"] } jts-test-runner = { path = "../jts-test-runner" } pretty_env_logger = "0.4" rand = "0.8.0" rand_distr = "0.4.3" - -### boolean-ops test deps wkt = "0.10.1" [[bench]] diff --git a/geo/src/algorithm/validation/geometry_collection.rs b/geo/src/algorithm/validation/geometry_collection.rs index e9318967b..de33377f4 100644 --- a/geo/src/algorithm/validation/geometry_collection.rs +++ b/geo/src/algorithm/validation/geometry_collection.rs @@ -45,9 +45,7 @@ impl Validation for GeometryCollection { mod tests { use super::*; use crate::algorithm::validation::{assert_validation_errors, InvalidLineString}; - use crate::{geometry::*, wkt}; - - use geos::Geom; + use crate::wkt; #[test] fn test_geometrycollection_contain_invalid_element() { @@ -58,6 +56,7 @@ mod tests { LINESTRING(0. 0.,0. 0.) ) ); + // the geos crate doesn't support converting geo::GeometryCollection to -> geos::Geom assert_validation_errors!( gc, vec![InvalidGeometryCollection::InvalidGeometry( @@ -65,21 +64,9 @@ mod tests { Box::new(InvalidGeometry::InvalidLineString( InvalidLineString::TooFewPoints )), - )] + )], + compare_with_geos: false ); - - let geoms = - gc.0.iter() - .map(|geometry| match geometry { - Geometry::Point(pt) => pt.try_into().unwrap(), - Geometry::LineString(ls) => ls.try_into().unwrap(), - _ => unreachable!(), - }) - .collect::>(); - - let geometrycollection_geos: geos::Geometry = - geos::Geometry::create_geometry_collection(geoms).unwrap(); - assert_eq!(gc.is_valid(), geometrycollection_geos.is_valid()); } #[test] diff --git a/geo/src/algorithm/validation/line.rs b/geo/src/algorithm/validation/line.rs index 563fe5973..cf44ff702 100644 --- a/geo/src/algorithm/validation/line.rs +++ b/geo/src/algorithm/validation/line.rs @@ -53,18 +53,21 @@ mod tests { #[test] fn test_line_valid() { let l = Line::new((0., 0.), (1., 1.)); - assert_valid!(l); + // GEOS doesn't have a Line type + assert_valid!(l, compare_with_geos: false); } #[test] fn test_line_invalid_not_finite_coords() { let l = Line::new((0., 0.), (f64::NEG_INFINITY, 0.)); - assert_validation_errors!(l, vec![InvalidLine::NonFiniteCoord(CoordIndex(1))]); + // GEOS doesn't have a Line type + assert_validation_errors!(l, vec![InvalidLine::NonFiniteCoord(CoordIndex(1))], compare_with_geos: false); } #[test] fn test_line_invalid_same_points() { let l = Line::new((0., 0.), (0., 0.)); - assert_validation_errors!(l, vec![InvalidLine::IdenticalCoords]); + // GEOS doesn't have a Line type + assert_validation_errors!(l, vec![InvalidLine::IdenticalCoords], compare_with_geos: false); } } diff --git a/geo/src/algorithm/validation/line_string.rs b/geo/src/algorithm/validation/line_string.rs index f826a32a8..c508f13cc 100644 --- a/geo/src/algorithm/validation/line_string.rs +++ b/geo/src/algorithm/validation/line_string.rs @@ -56,44 +56,30 @@ mod tests { use super::*; use crate::algorithm::validation::{assert_valid, assert_validation_errors}; use crate::wkt; - use geos::Geom; #[test] fn test_linestring_valid() { let ls = wkt!(LINESTRING(0. 0., 1. 1.)); - assert_valid!(ls); - - // Test that the linestring has the same validity status than its GEOS equivalent - let linestring_geos: geos::Geometry = (&ls).try_into().unwrap(); - assert_eq!(ls.is_valid(), linestring_geos.is_valid()); + assert_valid!(&ls); } #[test] fn test_linestring_valid_empty() { - let ls = wkt!(LINESTRING EMPTY); - assert_valid!(ls); - - let linestring_geos: geos::Geometry = (&ls).try_into().unwrap(); - assert_eq!(ls.is_valid(), linestring_geos.is_valid()); + let ls: LineString = wkt!(LINESTRING EMPTY); + assert_valid!(&ls); } #[test] fn test_linestring_invalid_too_few_points_without_duplicate() { let ls = wkt!(LINESTRING(0. 0.)); - assert_validation_errors!(ls, vec![InvalidLineString::TooFewPoints]); - - // Creating this linestring with geos fails (as soon as its creation is attempted) - let linestring_geos: geos::GResult = ls.try_into(); - assert!(linestring_geos.is_err()); + // NOTE: Rather than build an invalid LineString GEOS errors at construction time, so + // we can't compare with GEOS here. + assert_validation_errors!(&ls, vec![InvalidLineString::TooFewPoints], compare_with_geos: false); } #[test] fn test_linestring_invalid_too_few_points_with_duplicate() { let ls = wkt!(LINESTRING(0. 0.,0. 0.)); - assert_validation_errors!(ls, vec![InvalidLineString::TooFewPoints]); - - // Test that the linestring has the same validity status than its GEOS equivalent - let linestring_geos: geos::Geometry = (&ls).try_into().unwrap(); - assert_eq!(ls.is_valid(), linestring_geos.is_valid()); + assert_validation_errors!(&ls, vec![InvalidLineString::TooFewPoints]); } } diff --git a/geo/src/algorithm/validation/mod.rs b/geo/src/algorithm/validation/mod.rs index 0f664021e..b32d550a2 100644 --- a/geo/src/algorithm/validation/mod.rs +++ b/geo/src/algorithm/validation/mod.rs @@ -131,6 +131,24 @@ pub(crate) use test_macros::*; mod test_macros { macro_rules! assert_valid { ($to_validate:expr) => { + assert_valid!( + $to_validate, + compare_with_geos: true + ); + }; + ($to_validate:expr, compare_with_geos: true) => { + assert_valid!( + $to_validate, + compare_with_geos: false + ); + #[cfg(feature = "geos-tests")] + { + use geos::Geom; + let geos_geom: geos::Geometry = $to_validate.try_into().unwrap(); + assert!(geos_geom.is_valid(), "GEOS thought the geometry was invalid"); + } + }; + ($to_validate:expr, compare_with_geos: false) => { assert!( $to_validate.is_valid(), "Validation errors: {:?}", @@ -142,6 +160,26 @@ mod test_macros { macro_rules! assert_validation_errors { ($to_validate:expr, $errors:expr) => { + assert_validation_errors!( + $to_validate, + $errors, + compare_with_geos: true + ); + }; + ($to_validate:expr, $errors:expr, compare_with_geos: true) => { + assert_validation_errors!( + $to_validate, + $errors, + compare_with_geos: false + ); + #[cfg(feature = "geos-tests")] + { + use geos::Geom; + let geos_geom: geos::Geometry = $to_validate.try_into().unwrap(); + assert!(!geos_geom.is_valid(), "GEOS thought the geometry was valid"); + } + }; + ($to_validate:expr, $errors:expr, compare_with_geos: false) => { assert!(!$to_validate.is_valid()); assert!( !$errors.is_empty(), diff --git a/geo/src/algorithm/validation/multi_line_string.rs b/geo/src/algorithm/validation/multi_line_string.rs index d47905df3..15c994c89 100644 --- a/geo/src/algorithm/validation/multi_line_string.rs +++ b/geo/src/algorithm/validation/multi_line_string.rs @@ -46,7 +46,6 @@ mod tests { assert_valid, assert_validation_errors, InvalidLineString, InvalidMultiLineString, }; use crate::wkt; - use geos::Geom; #[test] fn test_multilinestring_valid() { @@ -56,11 +55,7 @@ mod tests { (3. 1.,4. 1.) ) ); - assert_valid!(mls); - - // Test that the linestring has the same validity status than its GEOS equivalent - let multilinestring_geos: geos::Geometry = (&mls).try_into().unwrap(); - assert_eq!(mls.is_valid(), multilinestring_geos.is_valid()); + assert_valid!(&mls); } #[test] @@ -74,15 +69,11 @@ mod tests { ) ); assert_validation_errors!( - mls, + &mls, vec![InvalidMultiLineString::InvalidLineString( GeometryIndex(1), InvalidLineString::TooFewPoints )] ); - - // Test that the linestring has the same validity status than its GEOS equivalent - let multilinestring_geos: geos::Geometry = (&mls).try_into().unwrap(); - assert_eq!(mls.is_valid(), multilinestring_geos.is_valid()); } } diff --git a/geo/src/algorithm/validation/multi_point.rs b/geo/src/algorithm/validation/multi_point.rs index a6422df6e..05dd15598 100644 --- a/geo/src/algorithm/validation/multi_point.rs +++ b/geo/src/algorithm/validation/multi_point.rs @@ -42,15 +42,11 @@ mod tests { use super::*; use crate::algorithm::validation::{assert_valid, assert_validation_errors}; use crate::{geometry::*, wkt}; - use geos::Geom; #[test] fn test_multipoint_valid() { let mp = wkt!(MULTIPOINT(0. 0.,1. 1.)); - assert_valid!(mp); - - let multipoint_geos: geos::Geometry = (&mp).try_into().unwrap(); - assert_eq!(mp.is_valid(), multipoint_geos.is_valid()); + assert_valid!(&mp); } #[test] @@ -60,15 +56,11 @@ mod tests { Point::new(f64::NAN, 1.), ]); assert_validation_errors!( - mp, + &mp, vec![ InvalidMultiPoint::InvalidPoint(GeometryIndex(0), InvalidPoint::NonFiniteCoord), InvalidMultiPoint::InvalidPoint(GeometryIndex(1), InvalidPoint::NonFiniteCoord) ] ); - - // Test that this multipoint has the same validity status than its GEOS equivalent - let multipoint_geos: geos::Geometry = (&mp).try_into().unwrap(); - assert_eq!(mp.is_valid(), multipoint_geos.is_valid()); } } diff --git a/geo/src/algorithm/validation/multi_polygon.rs b/geo/src/algorithm/validation/multi_polygon.rs index b6f24a325..fdd08cdf3 100644 --- a/geo/src/algorithm/validation/multi_polygon.rs +++ b/geo/src/algorithm/validation/multi_polygon.rs @@ -82,7 +82,6 @@ mod tests { use super::*; use crate::algorithm::validation::RingRole; use crate::wkt; - use geos::Geom; #[test] fn test_multipolygon_invalid() { @@ -102,7 +101,7 @@ mod tests { ) ); assert_validation_errors!( - multi_polygon, + &multi_polygon, vec![ InvalidMultiPolygon::InvalidPolygon( GeometryIndex(0), @@ -116,9 +115,5 @@ mod tests { ), ] ); - - // Test that the polygon has the same validity status than its GEOS equivalent - let multipolygon_geos: geos::Geometry = (&multi_polygon).try_into().unwrap(); - assert_eq!(multi_polygon.is_valid(), multipolygon_geos.is_valid()); } } diff --git a/geo/src/algorithm/validation/point.rs b/geo/src/algorithm/validation/point.rs index 6007c0fe5..8550ce11c 100644 --- a/geo/src/algorithm/validation/point.rs +++ b/geo/src/algorithm/validation/point.rs @@ -36,25 +36,17 @@ mod tests { use super::*; use crate::algorithm::validation::{assert_valid, assert_validation_errors}; use crate::Point; - use geos::Geom; #[test] fn test_point_valid() { let p = Point::new(0., 0.); assert_valid!(p); - - // Test that the point has the same validity status than its GEOS equivalent - let pt_geos: geos::Geometry = (&p).try_into().unwrap(); - assert_eq!(p.is_valid(), pt_geos.is_valid()); } #[test] fn test_point_validation_errors() { let p = Point::new(f64::NAN, f64::NAN); assert_validation_errors!(p, vec![InvalidPoint::NonFiniteCoord]); - - let pt_geos: geos::Geometry = (&p).try_into().unwrap(); - assert_eq!(p.is_valid(), pt_geos.is_valid()); } #[test] diff --git a/geo/src/algorithm/validation/polygon.rs b/geo/src/algorithm/validation/polygon.rs index 41e9e864f..9f736ab03 100644 --- a/geo/src/algorithm/validation/polygon.rs +++ b/geo/src/algorithm/validation/polygon.rs @@ -159,7 +159,6 @@ mod tests { use super::*; use crate::algorithm::validation::{assert_valid, assert_validation_errors}; use crate::wkt; - use geos::Geom; #[test] fn test_polygon_valid() { @@ -169,10 +168,6 @@ mod tests { POLYGON((0. 0., 1. 1., 0. 1.)) ); assert_valid!(&polygon); - - // Test that the polygon has the same validity status than its GEOS equivalent - let polygon_geos: geos::Geometry = (&polygon).try_into().unwrap(); - assert_eq!(polygon.is_valid(), polygon_geos.is_valid()); } #[test] @@ -187,10 +182,6 @@ mod tests { ) ); assert_valid!(&polygon); - - // Test that the polygon has the same validity status than its GEOS equivalent - let polygon_geos: geos::Geometry = (&polygon).try_into().unwrap(); - assert_eq!(polygon.is_valid(), polygon_geos.is_valid()); } #[test] @@ -205,10 +196,6 @@ mod tests { ) ); assert_valid!(&polygon); - - // Test that the polygon has the same validity status than its GEOS equivalent - let polygon_geos: geos::Geometry = (&polygon).try_into().unwrap(); - assert_eq!(polygon.is_valid(), polygon_geos.is_valid()); } #[test] @@ -224,23 +211,19 @@ mod tests { ); assert_validation_errors!( - polygon, + &polygon, vec![InvalidPolygon::IntersectingRingsOnALine( RingRole::Interior(0), RingRole::Interior(1) )] ); - - // Test that the polygon has the same validity status than its GEOS equivalent - let polygon_geos: geos::Geometry = (&polygon).try_into().unwrap(); - assert_eq!(polygon.is_valid(), polygon_geos.is_valid()); } #[test] fn test_polygon_invalid_interior_rings_crosses() { // The following polygon contains two interior rings that cross // each other (they share some common area), this is not valid. - let p = wkt!( + let polygon = wkt!( POLYGON( (0. 0., 4. 0., 4. 4., 0. 4., 0. 0.), (1. 2., 2. 1., 3. 2., 2. 3., 1. 2.), @@ -249,16 +232,12 @@ mod tests { ); assert_validation_errors!( - p, + &polygon, vec![InvalidPolygon::IntersectingRingsOnAnArea( RingRole::Interior(0), RingRole::Interior(1) )] ); - - // Test that the polygon has the same validity status than its GEOS equivalent - let polygon_geos: geos::Geometry = (&p).try_into().unwrap(); - assert_eq!(p.is_valid(), polygon_geos.is_valid()); } #[test] @@ -275,16 +254,12 @@ mod tests { ); assert_validation_errors!( - polygon, + &polygon, vec![InvalidPolygon::IntersectingRingsOnALine( RingRole::Exterior, RingRole::Interior(0) )] ); - - // Test that the polygon has the same validity status than its GEOS equivalent - let polygon_geos: geos::Geometry = (&polygon).try_into().unwrap(); - assert_eq!(polygon.is_valid(), polygon_geos.is_valid()); } #[test] @@ -294,13 +269,9 @@ mod tests { // to be a non-empty polygon let polygon = wkt!( POLYGON((0. 0., 1. 1.)) ); assert_validation_errors!( - polygon, + &polygon, vec![InvalidPolygon::TooFewPointsInRing(RingRole::Exterior)] ); - - // Test that the polygon has the same validity status than its GEOS equivalent - let polygon_geos: geos::Geometry = (&polygon).try_into().unwrap(); - assert_eq!(polygon.is_valid(), polygon_geos.is_valid()); } #[test] @@ -313,29 +284,21 @@ mod tests { ); assert_validation_errors!( - polygon, + &polygon, vec![InvalidPolygon::SelfIntersection(RingRole::Exterior)] ); - - // Test that the polygon has the same validity status than its GEOS equivalent - let polygon_geos: geos::Geometry = (&polygon).try_into().unwrap(); - assert_eq!(polygon.is_valid(), polygon_geos.is_valid()); } #[test] fn test_polygon_invalid_exterior_is_not_simple() { // The exterior ring of this polygon is not simple (i.e. it has a self-intersection) - let p = wkt!( + let polygon = wkt!( POLYGON((0. 0., 4. 0., 0. 2., 4. 2., 0. 0.)) ); assert_validation_errors!( - p, + &polygon, vec![InvalidPolygon::SelfIntersection(RingRole::Exterior)] ); - - // Test that the polygon has the same validity status than its GEOS equivalent - let polygon_geos: geos::Geometry = (&p).try_into().unwrap(); - assert_eq!(p.is_valid(), polygon_geos.is_valid()); } #[test] @@ -347,15 +310,11 @@ mod tests { ) ); assert_validation_errors!( - polygon, + &polygon, vec![InvalidPolygon::InteriorRingNotContainedInExteriorRing( RingRole::Interior(0) ),] ); - - // Test that the polygon has the same validity status than its GEOS equivalent - let polygon_geos: geos::Geometry = (&polygon).try_into().unwrap(); - assert_eq!(polygon.is_valid(), polygon_geos.is_valid()); } #[test] @@ -395,11 +354,5 @@ mod tests { RingRole::Interior(1) )] ); - - // Test that the polygons have the same validity status than their GEOS equivalents - let polygon_geos1: geos::Geometry = (&polygon_1).try_into().unwrap(); - let polygon_geos2: geos::Geometry = (&polygon_2).try_into().unwrap(); - assert_eq!(polygon_1.is_valid(), polygon_geos1.is_valid()); - assert_eq!(polygon_2.is_valid(), polygon_geos2.is_valid()); } } diff --git a/geo/src/algorithm/validation/triangle.rs b/geo/src/algorithm/validation/triangle.rs index 2075b54f4..f7322bda9 100644 --- a/geo/src/algorithm/validation/triangle.rs +++ b/geo/src/algorithm/validation/triangle.rs @@ -89,24 +89,28 @@ mod tests { #[test] fn test_triangle_valid() { let t = Triangle((0., 0.).into(), (0., 1.).into(), (0.5, 2.).into()); - assert_valid!(t); + // GEOS doesn't have a Triangle type + assert_valid!(t, compare_with_geos: false); } #[test] fn test_triangle_invalid_same_points() { let t = Triangle((0., 0.).into(), (0., 1.).into(), (0., 1.).into()); + // GEOS doesn't have a Triangle type assert_validation_errors!( t, vec![InvalidTriangle::IdenticalCoords( CoordIndex(1), CoordIndex(2) - )] + )], + compare_with_geos: false ); } #[test] fn test_triangle_invalid_points_collinear() { let t = Triangle((0., 0.).into(), (1., 1.).into(), (2., 2.).into()); - assert_validation_errors!(t, vec![InvalidTriangle::CollinearCoords]); + // GEOS doesn't have a Triangle type + assert_validation_errors!(t, vec![InvalidTriangle::CollinearCoords], compare_with_geos: false); } }