PreparedLineStringIntersects: Fix incorrect result for mixed-dim GC w…

…ith points (#774)
libgeos · Nov 1, 2023 · 5a9389e · 5a9389e
1 parent 43809f0
commit 5a9389e
Show file tree

Hide file tree

Showing 16 changed files with 139 additions and 7 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -12,6 +12,7 @@
   - GEOSClipByRect: Fix case with POINT EMPTY (GH-913, Mike Taves)
   - Remove undefined behaviour in use of null PrecisionModel (GH-931, Jeff Walton)
   - Fix InteriorPointPoint to handle empty elements (GH-977, Martin Davis)
+  - PreparedLineStringIntersects: Fix incorrect result with mixed-dim collection with points (GH-774, Dan Baston)
 
 ## Changes in 3.11.2
 2023-03-16

diff --git a/include/geos/geom/Geometry.h b/include/geos/geom/Geometry.h
@@ -342,6 +342,11 @@ class GEOS_DLL Geometry {
     /// Returns the dimension of this Geometry (0=point, 1=line, 2=surface)
     virtual Dimension::DimensionType getDimension() const = 0; //Abstract
 
+    /// Checks whether any component of this geometry has dimension d
+    virtual bool hasDimension(Dimension::DimensionType d) const {
+        return getDimension() == d;
+    }
+
     /// Checks whether this Geometry consists only of components having dimension d.
     virtual bool isDimensionStrict(Dimension::DimensionType d) const {
         return d == getDimension();

diff --git a/include/geos/geom/GeometryCollection.h b/include/geos/geom/GeometryCollection.h
@@ -109,6 +109,8 @@ class GEOS_DLL GeometryCollection : public Geometry {
      */
     Dimension::DimensionType getDimension() const override;
 
+    bool hasDimension(Dimension::DimensionType d) const override;
+
     bool isDimensionStrict(Dimension::DimensionType d) const override;
 
     /// Returns coordinate dimension.

diff --git a/include/geos/geom/MultiLineString.h b/include/geos/geom/MultiLineString.h
@@ -58,6 +58,10 @@ class GEOS_DLL MultiLineString: public GeometryCollection {
     /// Returns line dimension (1)
     Dimension::DimensionType getDimension() const override;
 
+    bool hasDimension(Dimension::DimensionType d) const override {
+        return d == Dimension::L;
+    }
+
     bool isDimensionStrict(Dimension::DimensionType d) const override {
         return d == Dimension::L;
     }

diff --git a/include/geos/geom/MultiPoint.h b/include/geos/geom/MultiPoint.h
@@ -63,6 +63,10 @@ class GEOS_DLL MultiPoint: public GeometryCollection {
         return d == Dimension::P;
     }
 
+    bool hasDimension(Dimension::DimensionType d) const override {
+        return d == Dimension::P;
+    }
+
     /// Returns Dimension::False (Point has no boundary)
     int getBoundaryDimension() const override;
 

diff --git a/include/geos/geom/MultiPolygon.h b/include/geos/geom/MultiPolygon.h
@@ -66,6 +66,10 @@ class GEOS_DLL MultiPolygon: public GeometryCollection {
     /// Returns surface dimension (2)
     Dimension::DimensionType getDimension() const override;
 
+    bool hasDimension(Dimension::DimensionType d) const override {
+        return d == Dimension::A;
+    }
+
     bool isDimensionStrict(Dimension::DimensionType d) const override {
         return d == Dimension::A;
     }

diff --git a/src/geom/GeometryCollection.cpp b/src/geom/GeometryCollection.cpp
@@ -137,6 +137,15 @@ GeometryCollection::isDimensionStrict(Dimension::DimensionType d) const {
             });
 }
 
+bool
+GeometryCollection::hasDimension(Dimension::DimensionType d) const {
+    return std::any_of(geometries.begin(),
+                       geometries.end(),
+                       [&d](const std::unique_ptr<Geometry>& g) {
+        return g->hasDimension(d);
+    });
+}
+
 int
 GeometryCollection::getBoundaryDimension() const
 {

diff --git a/src/geom/prep/PreparedLineStringIntersects.cpp b/src/geom/prep/PreparedLineStringIntersects.cpp
@@ -70,19 +70,14 @@ PreparedLineStringIntersects::intersects(const geom::Geometry* g) const
         return true;
     }
 
-    // For L/L case we are done
-    if(g->getDimension() == 1) {
-        return false;
-    }
-
     // For L/A case, need to check for proper inclusion of the target in the test
     if(g->getDimension() == 2
             &&	prepLine.isAnyTargetComponentInTest(g)) {
         return true;
     }
 
     // For L/P case, need to check if any points lie on line(s)
-    if(g->getDimension() == 0) {
+    if(g->hasDimension(Dimension::P)) {
         return isAnyTestPointInTarget(g);
     }
 

diff --git a/tests/unit/geom/GeometryCollectionTest.cpp b/tests/unit/geom/GeometryCollectionTest.cpp
@@ -154,4 +154,31 @@ void object::test<7>()
     ensure_equals(components.size(), 2u);
 }
 
+// Test of hasDimension()
+template<>
+template<>
+void object::test<8>
+()
+{
+    auto gc = readWKT("GEOMETRYCOLLECTION(POINT (1 1), LINESTRING (1 1, 2 2))");
+
+    ensure(gc->hasDimension(geos::geom::Dimension::P));
+    ensure(gc->hasDimension(geos::geom::Dimension::L));
+    ensure(!gc->hasDimension(geos::geom::Dimension::A));
+}
+
+
+// Test of hasDimension() for nested collection
+template<>
+template<>
+void object::test<9>
+()
+{
+    auto gc = readWKT("GEOMETRYCOLLECTION(POINT (1 1), GEOMETRYCOLLECTION(LINESTRING (1 1, 2 2), POLYGON((0 0, 0 1, 1 1, 0 0))))");
+
+    ensure(gc->hasDimension(geos::geom::Dimension::P));
+    ensure(gc->hasDimension(geos::geom::Dimension::L));
+    ensure(gc->hasDimension(geos::geom::Dimension::A));
+}
+
 } // namespace tut
diff --git a/tests/unit/geom/LineStringTest.cpp b/tests/unit/geom/LineStringTest.cpp
@@ -573,6 +573,18 @@ void object::test<31>
 
 }
 
+// Test of hasDimension()
+template<>
+template<>
+void object::test<32>
+()
+{
+    auto line = reader_.read<geos::geom::LineString>("LINESTRING (0 0, 10 10)");
+
+    ensure(!line->hasDimension(geos::geom::Dimension::P));
+    ensure(line->hasDimension(geos::geom::Dimension::L));
+    ensure(!line->hasDimension(geos::geom::Dimension::A));
+}
 
 } // namespace tut
 
diff --git a/tests/unit/geom/MultiLineStringTest.cpp b/tests/unit/geom/MultiLineStringTest.cpp
@@ -63,5 +63,18 @@ void object::test<3>
     ensure(!mls_->isDimensionStrict(geos::geom::Dimension::A));
 }
 
+
+// Test of hasDimension()
+template<>
+template<>
+void object::test<4>
+()
+{
+    ensure(!mls_->hasDimension(geos::geom::Dimension::P));
+    ensure(mls_->hasDimension(geos::geom::Dimension::L));
+    ensure(!mls_->hasDimension(geos::geom::Dimension::A));
+}
+
+
 } // namespace tut
 
diff --git a/tests/unit/geom/MultiPointTest.cpp b/tests/unit/geom/MultiPointTest.cpp
@@ -417,5 +417,16 @@ void object::test<31>
     ensure(!empty_mp_->isDimensionStrict(geos::geom::Dimension::L));
 }
 
+// Test of hasDimension()
+template<>
+template<>
+void object::test<32>
+()
+{
+    ensure(mp_->hasDimension(geos::geom::Dimension::P));
+    ensure(!mp_->hasDimension(geos::geom::Dimension::L));
+    ensure(!mp_->hasDimension(geos::geom::Dimension::A));
+}
+
 } // namespace tut
 
diff --git a/tests/unit/geom/MultiPolygonTest.cpp b/tests/unit/geom/MultiPolygonTest.cpp
@@ -62,4 +62,15 @@ void object::test<3>
     ensure(!empty_mp_->isDimensionStrict(geos::geom::Dimension::L));
 }
 
+// Test of hasDimension()
+template<>
+template<>
+void object::test<4>
+()
+{
+    ensure(!mp_->hasDimension(geos::geom::Dimension::P));
+    ensure(!mp_->hasDimension(geos::geom::Dimension::L));
+    ensure(mp_->hasDimension(geos::geom::Dimension::A));
+}
+
 } // namespace tut
diff --git a/tests/unit/geom/PointTest.cpp b/tests/unit/geom/PointTest.cpp
@@ -17,6 +17,7 @@
 #include <memory>
 #include <string>
 
+constexpr int MAX_TESTS = 100;
 
 namespace tut {
 //
@@ -58,7 +59,7 @@ struct test_point_data {
     }
 };
 
-typedef test_group<test_point_data> group;
+typedef test_group<test_point_data, MAX_TESTS> group;
 typedef group::object object;
 
 group test_point_group("geos::geom::Point");
@@ -600,5 +601,16 @@ void object::test<46>
     ensure("point->getCoordinateDimension() == 2", point->getCoordinateDimension() == 2);
 }
 
+// Test of hasDimension()
+template<>
+template<>
+void object::test<47>
+()
+{
+    ensure(point_->hasDimension(geos::geom::Dimension::P));
+    ensure(!point_->hasDimension(geos::geom::Dimension::L));
+    ensure(!point_->hasDimension(geos::geom::Dimension::A));
+}
+
 } // namespace tut
 
diff --git a/tests/unit/geom/PolygonTest.cpp b/tests/unit/geom/PolygonTest.cpp
@@ -652,4 +652,15 @@ void object::test<44>()
     ensure_equals(holes.size(), 1u);
 }
 
+// Test of hasDimension()
+template<>
+template<>
+void object::test<45>
+()
+{
+    ensure(!poly_->hasDimension(geos::geom::Dimension::P));
+    ensure(!poly_->hasDimension(geos::geom::Dimension::L));
+    ensure(poly_->hasDimension(geos::geom::Dimension::A));
+}
+
 } // namespace tut
diff --git a/tests/xmltester/tests/general/TestPreparedPredicatesWithGeometryCollection.xml b/tests/xmltester/tests/general/TestPreparedPredicatesWithGeometryCollection.xml
@@ -76,6 +76,17 @@
 <test>  <op name="intersects"     arg1="A" arg2="B">   true  </op> </test>
 </case>
 
+<case>
+  <desc>LineString against GC, with point at endpoint
+  </desc>
+  <a>
+  LINESTRING (0 0, 1 1)
+  </a>
+  <b>
+    GEOMETRYCOLLECTION (POINT (1 1), LINESTRING (2 2, 3 3))
+  </b>
+<test>  <op name="intersects"     arg1="A" arg2="B">   true  </op> </test>
+</case>
 
 
 </run>