Merge pull request #39790 from nyalldawson/vcloarec-fixGeometryValidator

Fix crash in geometry validator
qgis · Nov 4, 2020 · a7d4bdd · a7d4bdd
2 parents 174d30c + c7ab2d2
commit a7d4bdd
Show file tree

Hide file tree

Showing 3 changed files with 192 additions and 14 deletions.
diff --git a/src/core/qgsgeometryvalidator.cpp b/src/core/qgsgeometryvalidator.cpp
@@ -41,7 +41,7 @@ void QgsGeometryValidator::stop()
   mStop = true;
 }
 
-void QgsGeometryValidator::checkRingIntersections( int p0, int i0, const QgsLineString *ring0, int p1, int i1, const QgsLineString *ring1 )
+void QgsGeometryValidator::checkRingIntersections( int partIndex0, int ringIndex0, const QgsLineString *ring0, int partIndex1, int ringIndex1, const QgsLineString *ring1 )
 {
   for ( int i = 0; !mStop && i < ring0->numPoints() - 1; i++ )
   {
@@ -65,14 +65,13 @@ void QgsGeometryValidator::checkRingIntersections( int p0, int i0, const QgsLine
         {
           d = -distLine2Point( ring1XAtj, ring1YAtj, w.perpVector(), sX, sY );
           if ( d > 0 && d < w.length() &&
-               ring0[i + 1] != ring1[j + 1] && ring0[i + 1] != ring1[j] &&
-               ring0[i + 0] != ring1[j + 1] && ring0[i + 0] != ring1[j] )
+               ring0->pointN( i + 1 ) != ring1->pointN( j + 1 ) && ring0->pointN( i + 1 ) != ring1->pointN( j ) &&
+               ring0->pointN( i + 0 ) != ring1->pointN( j + 1 ) && ring0->pointN( i + 0 ) != ring1->pointN( j ) )
           {
-            QString msg = QObject::tr( "segment %1 of ring %2 of polygon %3 intersects segment %4 of ring %5 of polygon %6 at %7, %8" )
-                          .arg( i0 ).arg( i ).arg( p0 )
-                          .arg( i1 ).arg( j ).arg( p1 )
-                          .arg( sX ).arg( sY );
-            QgsDebugMsg( msg );
+            const QString msg = QObject::tr( "segment %1 of ring %2 of polygon %3 intersects segment %4 of ring %5 of polygon %6 at %7, %8" )
+                                .arg( i ).arg( ringIndex0 ).arg( partIndex0 )
+                                .arg( j ).arg( ringIndex1 ).arg( partIndex1 )
+                                .arg( sX ).arg( sY );
             emit errorFound( QgsGeometry::Error( msg, QgsPointXY( sX, sY ) ) );
             mErrorCount++;
           }
@@ -214,14 +213,14 @@ void QgsGeometryValidator::validatePolyline( int i, const QgsLineString *line, b
   }
 }
 
-void QgsGeometryValidator::validatePolygon( int idx, const QgsPolygon *polygon )
+void QgsGeometryValidator::validatePolygon( int partIndex, const QgsPolygon *polygon )
 {
   // check if holes are inside polygon
   for ( int i = 0; !mStop && i < polygon->numInteriorRings(); ++i )
   {
     if ( !ringInRing( static_cast< const QgsLineString * >( polygon->interiorRing( i ) ), static_cast< const QgsLineString * >( polygon->exteriorRing() ) ) )
     {
-      QString msg = QObject::tr( "ring %1 of polygon %2 not in exterior ring" ).arg( i + 1 ).arg( idx );
+      QString msg = QObject::tr( "ring %1 of polygon %2 not in exterior ring" ).arg( i + 1 ).arg( partIndex );
       QgsDebugMsg( msg );
       emit errorFound( QgsGeometry::Error( msg ) );
       mErrorCount++;
@@ -233,8 +232,8 @@ void QgsGeometryValidator::validatePolygon( int idx, const QgsPolygon *polygon )
   {
     for ( int j = i + 1; !mStop && j < polygon->numInteriorRings(); j++ )
     {
-      checkRingIntersections( idx, i + 1, qgsgeometry_cast< QgsLineString * >( polygon->interiorRing( i ) ),
-                              idx, j + 1, qgsgeometry_cast< QgsLineString * >( polygon->interiorRing( j ) ) );
+      checkRingIntersections( partIndex, i + 1, qgsgeometry_cast< QgsLineString * >( polygon->interiorRing( i ) ),
+                              partIndex, j + 1, qgsgeometry_cast< QgsLineString * >( polygon->interiorRing( j ) ) );
     }
   }
 

diff --git a/src/core/qgsgeometryvalidator.h b/src/core/qgsgeometryvalidator.h
@@ -71,8 +71,8 @@ class CORE_EXPORT QgsGeometryValidator : public QThread
 
   private:
     void validatePolyline( int i, const QgsLineString *line, bool ring = false );
-    void validatePolygon( int i, const QgsPolygon *polygon );
-    void checkRingIntersections( int p0, int i0, const QgsLineString *ring0, int p1, int i1, const QgsLineString *ring1 );
+    void validatePolygon( int partIndex, const QgsPolygon *polygon );
+    void checkRingIntersections( int partIndex0, int ringIndex0, const QgsLineString *ring0, int partIndex1, int ringIndex1, const QgsLineString *ring1 );
     double distLine2Point( double px, double py, QgsVector v, double qX, double qY );
     bool intersectLines( double px, double py, QgsVector v, double qx, double qy, QgsVector w, double &sX, double &sY );
     bool ringInRing( const QgsLineString *inside, const QgsLineString *outside );

diff --git a/tests/src/python/test_qgsgeometryvalidator.py b/tests/src/python/test_qgsgeometryvalidator.py
@@ -83,6 +83,185 @@ def test_linestring_duplicate_nodes(self):
         self.assertEqual(spy[2][0].where(), QgsPointXY(1, 1))
         self.assertEqual(spy[2][0].what(), 'line 1 contains 3 duplicate nodes starting at vertex 1')
 
+    def test_ring_intersections(self):
+        # no intersections
+        g = QgsGeometry.fromWkt("Polygon ((0 0, 10 0, 10 10, 0 10, 0 0),(1 1, 5 1, 1 9, 1 1), (6 9, 2 9, 6 1, 6 9))")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+
+        self.assertEqual(len(spy), 0)
+
+        # two interior rings intersecting
+        g = QgsGeometry.fromWkt("Polygon ((0 0, 10 0, 10 10, 0 10, 0 0),(1 1, 5 1, 1 9, 1 1), (2 2, 5 2, 2 9, 2 2))")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+
+        self.assertEqual(len(spy), 2)
+
+        self.assertEqual(spy[0][0].where(), QgsPointXY(4.5, 2))
+        self.assertEqual(spy[0][0].what(), 'segment 1 of ring 1 of polygon 0 intersects segment 0 of ring 2 of polygon 0 at 4.5, 2')
+
+        self.assertEqual(spy[1][0].where(), QgsPointXY(2, 7))
+        self.assertEqual(spy[1][0].what(), 'segment 1 of ring 1 of polygon 0 intersects segment 2 of ring 2 of polygon 0 at 2, 7')
+
+    def test_line_vertices(self):
+        # valid line
+        g = QgsGeometry.fromWkt("LineString (0 0, 10 0)")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+        self.assertEqual(len(spy), 0)
+
+        # not enough vertices
+        g = QgsGeometry.fromWkt("LineString (1 0)")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+        self.assertEqual(len(spy), 1)
+
+        self.assertEqual(spy[0][0].where(), QgsPointXY())
+        self.assertEqual(spy[0][0].what(), 'line 0 with less than two points')
+
+        g = QgsGeometry.fromWkt("LineString ()")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+        self.assertEqual(len(spy), 1)
+
+        self.assertEqual(spy[0][0].where(), QgsPointXY())
+        self.assertEqual(spy[0][0].what(), 'line 0 with less than two points')
+
+    def test_ring_vertex_count(self):
+        # valid ring
+        g = QgsGeometry.fromWkt("Polygon ((0 0, 10 0, 10 10, 0 0))")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+        self.assertEqual(len(spy), 0)
+
+        # not enough vertices
+        g = QgsGeometry.fromWkt("Polygon ((0 0, 10 0, 10 10))")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+        self.assertEqual(len(spy), 1)
+
+        self.assertEqual(spy[0][0].where(), QgsPointXY())
+        self.assertEqual(spy[0][0].what(), 'ring 0 with less than four points')
+
+        g = QgsGeometry.fromWkt("Polygon ((0 0, 10 0, 0 0))")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+        self.assertEqual(len(spy), 1)
+
+        self.assertEqual(spy[0][0].where(), QgsPointXY())
+        self.assertEqual(spy[0][0].what(), 'ring 0 with less than four points')
+
+        g = QgsGeometry.fromWkt("Polygon ((0 0, 10 0))")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+        self.assertEqual(len(spy), 1)
+
+        self.assertEqual(spy[0][0].where(), QgsPointXY())
+        self.assertEqual(spy[0][0].what(), 'ring 0 with less than four points')
+
+        g = QgsGeometry.fromWkt("Polygon ((0 0))")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+        self.assertEqual(len(spy), 1)
+
+        self.assertEqual(spy[0][0].where(), QgsPointXY())
+        self.assertEqual(spy[0][0].what(), 'ring 0 with less than four points')
+
+        g = QgsGeometry.fromWkt("Polygon (())")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+        self.assertEqual(len(spy), 0)
+
+        g = QgsGeometry.fromWkt("Polygon ((0 0, 10 0, 10 10, 0 0),(1 1, 2 1, 2 2))")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+        self.assertEqual(len(spy), 1)
+
+        self.assertEqual(spy[0][0].where(), QgsPointXY())
+        self.assertEqual(spy[0][0].what(), 'ring 1 with less than four points')
+
+        g = QgsGeometry.fromWkt("Polygon ((0 0, 10 0, 10 10, 0 0),(1 1, 2 1, 2 2, 1 1))")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+        self.assertEqual(len(spy), 0)
+
+        g = QgsGeometry.fromWkt("Polygon ((0 0, 10 0, 10 10, 0 10, 0 0),(1 1, 2 1, 2 2, 1 1),(3 3, 3 4, 4 4))")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+        self.assertEqual(len(spy), 1)
+
+        self.assertEqual(spy[0][0].where(), QgsPointXY())
+        self.assertEqual(spy[0][0].what(), 'ring 2 with less than four points')
+
+    def test_ring_closed(self):
+        # valid ring
+        g = QgsGeometry.fromWkt("Polygon ((0 0, 10 0, 10 10, 0 0))")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+        self.assertEqual(len(spy), 0)
+
+        # not closed
+        g = QgsGeometry.fromWkt("Polygon ((0 0, 10 0, 10 10, 1 1))")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+        self.assertEqual(len(spy), 1)
+
+        self.assertEqual(spy[0][0].where(), QgsPointXY())
+        self.assertEqual(spy[0][0].what(), 'ring 0 not closed')
+
+        g = QgsGeometry.fromWkt("Polygon ((0 0, 10 0, 10 10, 0 0),(1 1, 2 1, 2 2, 1.1 1))")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+        self.assertEqual(len(spy), 1)
+
+        self.assertEqual(spy[0][0].where(), QgsPointXY())
+        self.assertEqual(spy[0][0].what(), 'ring 1 not closed')
+
+        g = QgsGeometry.fromWkt("Polygon ((0 0, 10 0, 10 10, 0 10, 0 0),(1 1, 2 1, 2 2, 1 1),(3 3, 3 4, 4 4, 3.1 3))")
+
+        validator = QgsGeometryValidator(g)
+        spy = QSignalSpy(validator.errorFound)
+        validator.run()
+        self.assertEqual(len(spy), 1)
+
+        self.assertEqual(spy[0][0].where(), QgsPointXY())
+        self.assertEqual(spy[0][0].what(), 'ring 2 not closed')
+
 
 if __name__ == '__main__':
     unittest.main()