From ea46eefa48b7eaaeb1d7fafb0610d5b1db0b35f9 Mon Sep 17 00:00:00 2001 From: Paul Ramsey Date: Tue, 31 Oct 2023 10:40:33 -0700 Subject: [PATCH 1/3] Skip over testing empty distances for mixed collections. Closes #979 --- src/operation/distance/DistanceOp.cpp | 16 ++++-- .../operation/distance/DistanceOpTest.cpp | 56 +++++++++++++++++++ 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/src/operation/distance/DistanceOp.cpp b/src/operation/distance/DistanceOp.cpp index 2f3cb66638..952c68d96c 100644 --- a/src/operation/distance/DistanceOp.cpp +++ b/src/operation/distance/DistanceOp.cpp @@ -358,6 +358,10 @@ DistanceOp::computeMinDistanceLines( { for(const LineString* line0 : lines0) { for(const LineString* line1 : lines1) { + + if (line0->isEmpty() || line1->isEmpty()) + continue; + computeMinDistance(line0, line1, locGeom); if(minDistance <= terminateDistance) { return; @@ -374,13 +378,11 @@ DistanceOp::computeMinDistancePoints( std::array, 2> & locGeom) { for(const Point* pt0 : points0) { - if (pt0->isEmpty()) { - continue; - } for(const Point* pt1 : points1) { - if (pt1->isEmpty()) { + + if (pt1->isEmpty() || pt0->isEmpty()) continue; - } + double dist = pt0->getCoordinate()->distance(*(pt1->getCoordinate())); #if GEOS_DEBUG @@ -414,6 +416,10 @@ DistanceOp::computeMinDistanceLinesPoints( { for(const LineString* line : lines) { for(const Point* pt : points) { + + if (line->isEmpty() || pt->isEmpty()) + continue; + computeMinDistance(line, pt, locGeom); if(minDistance <= terminateDistance) { return; diff --git a/tests/unit/operation/distance/DistanceOpTest.cpp b/tests/unit/operation/distance/DistanceOpTest.cpp index 9b862e377a..2bd4163b53 100644 --- a/tests/unit/operation/distance/DistanceOpTest.cpp +++ b/tests/unit/operation/distance/DistanceOpTest.cpp @@ -565,6 +565,11 @@ void object::test<21>() ensure_equals(g1->distance(g2.get()), 1.9996999774966246); } +// +// Variations on a theme: testing EMPTY and collections with EMPTY +// + +// Ignoring empty component template<> template<> void object::test<22>() @@ -576,6 +581,57 @@ void object::test<22>() ensure_equals(g2->distance(g1.get()), 1); } +// Empty is same as empty so zero...? +template<> +template<> +void object::test<23>() +{ + auto g1 = wktreader.read("POINT EMPTY"); + auto g2 = wktreader.read("LINESTRING EMPTY"); + + ensure(g1 != nullptr && g2 != nullptr); + ensure_equals(g1->distance(g2.get()), 0); + ensure_equals(g2->distance(g1.get()), 0); +} + +template<> +template<> +void object::test<24>() +{ + auto g1 = wktreader.read("GEOMETRYCOLLECTION(POINT EMPTY, LINESTRING EMPTY)"); + auto g2 = wktreader.read("LINESTRING EMPTY"); + + ensure(g1 != nullptr && g2 != nullptr); + ensure_equals(g1->distance(g2.get()), 0); + ensure_equals(g2->distance(g1.get()), 0); +} + +// But ignore empty if there's a real distance? +template<> +template<> +void object::test<25>() +{ + auto g1 = wktreader.read("GEOMETRYCOLLECTION(LINESTRING EMPTY, POINT(2 1))"); + auto g2 = wktreader.read("POINT(1 1)"); + + ensure(g1 != nullptr && g2 != nullptr); + ensure_equals(g1->distance(g2.get()), 1); + ensure_equals(g2->distance(g1.get()), 1); +} + +template<> +template<> +void object::test<26>() +{ + auto g1 = wktreader.read("GEOMETRYCOLLECTION(POINT(-2 0), POINT EMPTY)"); + auto g2 = wktreader.read("GEOMETRYCOLLECTION(POINT(1 0),LINESTRING(0 0,1 0))"); + + ensure(g1 != nullptr && g2 != nullptr); + ensure_equals(g1->distance(g2.get()), 2); + ensure_equals(g2->distance(g1.get()), 2); +} + + // TODO: finish the tests by adding: // LINESTRING - *all* // MULTILINESTRING - *all* From 90cddc0647d0e67fbab0f8ebd4d6c7e704ab3e2e Mon Sep 17 00:00:00 2001 From: Paul Ramsey Date: Tue, 31 Oct 2023 10:42:40 -0700 Subject: [PATCH 2/3] Add a test case for EMPTY/real pair (returns zero distance) --- tests/unit/operation/distance/DistanceOpTest.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/unit/operation/distance/DistanceOpTest.cpp b/tests/unit/operation/distance/DistanceOpTest.cpp index 2bd4163b53..9df481a397 100644 --- a/tests/unit/operation/distance/DistanceOpTest.cpp +++ b/tests/unit/operation/distance/DistanceOpTest.cpp @@ -631,6 +631,18 @@ void object::test<26>() ensure_equals(g2->distance(g1.get()), 2); } +template<> +template<> +void object::test<27>() +{ + auto g1 = wktreader.read("GEOMETRYCOLLECTION(POINT EMPTY)"); + auto g2 = wktreader.read("GEOMETRYCOLLECTION(POINT(1 0))"); + + ensure(g1 != nullptr && g2 != nullptr); + ensure_equals(g1->distance(g2.get()), 0); + ensure_equals(g2->distance(g1.get()), 0); +} + // TODO: finish the tests by adding: // LINESTRING - *all* From 28bf7e0fbccfaf784e2223d7c7b24b637ff72d0b Mon Sep 17 00:00:00 2001 From: Paul Ramsey Date: Thu, 2 Nov 2023 14:30:56 -0700 Subject: [PATCH 3/3] Add XML tests from JTS --- .../xmltester/tests/general/TestDistance.xml | 67 ++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) diff --git a/tests/xmltester/tests/general/TestDistance.xml b/tests/xmltester/tests/general/TestDistance.xml index 4ce6d0fede..3835c919da 100644 --- a/tests/xmltester/tests/general/TestDistance.xml +++ b/tests/xmltester/tests/general/TestDistance.xml @@ -6,6 +6,7 @@ POINT(10 10) POINT EMPTY 0.0 + 0.0 @@ -13,6 +14,31 @@ POINT(10 10) POINT (10 0) 10.0 + 10.0 + + + + PP - point to multipoint + POINT(10 10) + MULTIPOINT ((10 0), (30 30)) + 10.0 + 10.0 + + + + PP - point to multipoint with empty element + POINT(10 10) + MULTIPOINT ((10 0), EMPTY) + 10.0 + 10.0 + + + + LL - line to empty line + LINESTRING (0 0, 0 10) + LINESTRING EMPTY + 0.0 + 0.0 @@ -20,6 +46,31 @@ LINESTRING (0 0, 0 10) LINESTRING (10 0, 10 10) 10.0 + 10.0 + + + + LL - line to multiline + LINESTRING (0 0, 0 10) + MULTILINESTRING ((10 0, 10 10), (50 50, 60 60)) + 10.0 + 10.0 + + + + LL - line to multiline with empty element + LINESTRING (0 0, 0 10) + MULTILINESTRING ((10 0, 10 10), EMPTY) + 10.0 + 10.0 + + + + PA - point to empty polygon + POINT (240 160) + POLYGON EMPTY + 0.0 + 0.0 @@ -27,6 +78,7 @@ POINT (240 160) POLYGON ((100 260, 340 180, 100 60, 180 160, 100 260)) 0.0 + 0.0 @@ -34,6 +86,7 @@ LINESTRING (40 300, 280 220, 60 160, 140 60) LINESTRING (140 360, 260 280, 240 120, 120 160) 0.0 + 0.0 @@ -41,6 +94,7 @@ POLYGON ((60 260, 260 180, 100 60, 60 160, 60 260)) POLYGON ((220 280, 120 160, 300 60, 360 220, 220 280)) 0.0 + 0.0 @@ -48,6 +102,7 @@ POLYGON ((100 320, 60 120, 240 180, 200 260, 100 320)) POLYGON ((420 320, 280 260, 400 100, 420 320)) 71.55417527999327 + 71.55417527999327 @@ -55,13 +110,23 @@ MULTIPOLYGON (((40 240, 160 320, 40 380, 40 240)), ((100 240, 240 60, 40 40, 100 240))) MULTIPOLYGON (((220 280, 120 160, 300 60, 360 220, 220 280)), ((240 380, 280 300, 420 340, 240 380))) 0.0 + 0.0 - mAmA - multipolygon with empty component + mAmA - multipolygon with empty element MULTIPOLYGON (EMPTY, ((98 200, 200 200, 200 99, 98 99, 98 200))) POLYGON ((300 200, 400 200, 400 100, 300 100, 300 200)) 100.0 + 100.0 + + + + GCGC - geometry collections with mixed dimensions + GEOMETRYCOLLECTION (LINESTRING (10 10, 50 10), POINT (90 10)) + GEOMETRYCOLLECTION (POLYGON ((90 20, 60 20, 60 50, 90 50, 90 20)), LINESTRING (10 50, 30 70)) + 10.0 + 10.0