Remove invalid ring handling from filterVertices, move to removeInvalidRings()

Author: nyalldawson

python/core/auto_generated/geometry/qgscurvepolygon.sip.in

@@ -122,6 +122,15 @@ area will be removed.
 
 .. seealso:: :py:func:`removeInteriorRing`
 
+.. versionadded:: 3.0
+%End
+
+    void removeInvalidRings();
+%Docstring
+Removes any interior rings which are not valid from the polygon.
+
+For example, this removes unclosed rings and rings with less than 4 vertices.
+
 .. versionadded:: 3.0
 %End
 

src/core/geometry/qgsabstractgeometry.h

@@ -573,8 +573,7 @@ class CORE_EXPORT QgsAbstractGeometry
      * Filters the vertices from the geometry in place, removing any which do not return true for the \a filter function
      * check. Has no meaning when called on a single point geometry.
      *
-     * Depending on the \a filter used, this may result in an invalid geometry. However, CurvePolygon rings which are no longer
-     * valid rings will be automatically removed after filtering.
+     * Depending on the \a filter used, this may result in an invalid geometry.
      *
      * \note Not available in Python bindings
      * \since QGIS 3.2

src/core/geometry/qgscurvepolygon.cpp

@@ -703,6 +703,25 @@ void QgsCurvePolygon::removeInteriorRings( double minimumAllowedArea )
   clearCache();
 }
 
+void QgsCurvePolygon::removeInvalidRings()
+{
+  QVector<QgsCurve *> validRings;
+  validRings.reserve( mInteriorRings.size() );
+  for ( QgsCurve *curve : qgis::as_const( mInteriorRings ) )
+  {
+    if ( !curve->isRing() )
+    {
+      // remove invalid rings
+      delete curve;
+    }
+    else
+    {
+      validRings << curve;
+    }
+  }
+  mInteriorRings = validRings;
+}
+
 void QgsCurvePolygon::draw( QPainter &p ) const
 {
   if ( !mExteriorRing )
@@ -1179,23 +1198,10 @@ void QgsCurvePolygon::filterVertices( const std::function<bool ( const QgsPoint
   if ( mExteriorRing )
     mExteriorRing->filterVertices( filter );
 
-  QVector<QgsCurve *> filteredRings;
-  filteredRings.reserve( mInteriorRings.size() );
   for ( QgsCurve *curve : qgis::as_const( mInteriorRings ) )
   {
     curve->filterVertices( filter );
-
-    if ( !curve->isRing() )
-    {
-      // remove invalid rings
-      delete curve;
-    }
-    else
-    {
-      filteredRings << curve;
-    }
   }
-  mInteriorRings = filteredRings;
   clearCache();
 }
 

src/core/geometry/qgscurvepolygon.h

@@ -109,6 +109,15 @@ class CORE_EXPORT QgsCurvePolygon: public QgsSurface
      */
     void removeInteriorRings( double minimumAllowedArea = -1 );
 
+    /**
+     * Removes any interior rings which are not valid from the polygon.
+     *
+     * For example, this removes unclosed rings and rings with less than 4 vertices.
+     *
+     * \since QGIS 3.0
+     */
+    void removeInvalidRings();
+
     void draw( QPainter &p ) const override;
     void transform( const QgsCoordinateTransform &ct, QgsCoordinateTransform::TransformDirection d = QgsCoordinateTransform::ForwardTransform, bool transformZ = false ) override SIP_THROW( QgsCsException );
     void transform( const QTransform &t, double zTranslate = 0.0, double zScale = 1.0, double mTranslate = 0.0, double mScale = 1.0 ) override;

src/core/geometry/qgsgeometry.h

@@ -1533,8 +1533,7 @@ class CORE_EXPORT QgsGeometry
      * Filters the vertices from the geometry in place, removing any which do not return true for the \a filter function
      * check. Has no effect when called on a single point geometry.
      *
-     * Depending on the \a filter used, this may result in an invalid geometry. However, CurvePolygon rings which are no longer
-     * valid rings will be automatically removed after filtering.
+     * Depending on the \a filter used, this may result in an invalid geometry.
      *
      * \note Not available in Python bindings
      * \since QGIS 3.2

tests/src/core/testqgsgeometry.cpp

@@ -6317,7 +6317,22 @@ void TestQgsGeometry::polygon()
   filterPolygonRing.setPoints( QgsPointSequence() << QgsPoint( 1, 2, 5, 6, QgsWkbTypes::PointZM ) << QgsPoint( 11.01, 2.01, 15, 16, QgsWkbTypes::PointZM ) << QgsPoint( 11, 2.01, 25, 26, QgsWkbTypes::PointZM ) << QgsPoint( 1, 2, 5, 6, QgsWkbTypes::PointZM ) );
   filterPolygon.addInteriorRing( filterPolygonRing.clone() );
   filterPolygon.filterVertices( filter );
-  QCOMPARE( filterPolygon.asWkt( 2 ), QStringLiteral( "PolygonZM ((11 2 3 4, 11 12 13 14, 11 22 23 24, 11 2 3 4),(10 2 5 6, 11.01 2.01 15 16, 11 2.01 25 26, 11 2 5 6, 10 2 5 6))" ) );
+  QCOMPARE( filterPolygon.asWkt( 2 ), QStringLiteral( "PolygonZM ((11 2 3 4, 11 12 13 14, 11 22 23 24, 11 2 3 4),(10 2 5 6, 11.01 2.01 15 16, 11 2.01 25 26, 11 2 5 6, 10 2 5 6),(11.01 2.01 15 16, 11 2.01 25 26))" ) );
+
+  // remove invalid rings
+  QgsPolygon invalidRingPolygon;
+  invalidRingPolygon.removeInvalidRings(); // no crash
+  QgsLineString removeInvalidPolygonRing;
+  removeInvalidPolygonRing.setPoints( QgsPointSequence() << QgsPoint( 11, 2, 3, 4, QgsWkbTypes::PointZM ) << QgsPoint( 4, 12, 13, 14, QgsWkbTypes::PointZM ) << QgsPoint( 11, 12, 13, 14, QgsWkbTypes::PointZM ) << QgsPoint( 11, 22, 23, 24, QgsWkbTypes::PointZM ) << QgsPoint( 11, 2, 3, 4, QgsWkbTypes::PointZM ) );
+  invalidRingPolygon.setExteriorRing( removeInvalidPolygonRing.clone() );
+  invalidRingPolygon.removeInvalidRings();
+  QCOMPARE( invalidRingPolygon.asWkt(), QStringLiteral( "PolygonZM ((11 2 3 4, 4 12 13 14, 11 12 13 14, 11 22 23 24, 11 2 3 4))" ) );
+  removeInvalidPolygonRing.setPoints( QgsPointSequence() << QgsPoint( 10, 2, 5, 6, QgsWkbTypes::PointZM )  << QgsPoint( 11, 2, 5, 6, QgsWkbTypes::PointZM ) << QgsPoint( 10, 2, 5, 6, QgsWkbTypes::PointZM ) );
+  invalidRingPolygon.addInteriorRing( removeInvalidPolygonRing.clone() );
+  removeInvalidPolygonRing.setPoints( QgsPointSequence() << QgsPoint( 1, 2, 5, 6, QgsWkbTypes::PointZM ) << QgsPoint( 11.01, 2.01, 15, 16, QgsWkbTypes::PointZM ) << QgsPoint( 11, 2.01, 25, 26, QgsWkbTypes::PointZM ) << QgsPoint( 1, 2, 5, 6, QgsWkbTypes::PointZM ) );
+  invalidRingPolygon.addInteriorRing( removeInvalidPolygonRing.clone() );
+  invalidRingPolygon.removeInvalidRings();
+  QCOMPARE( invalidRingPolygon.asWkt( 2 ), QStringLiteral( "PolygonZM ((11 2 3 4, 4 12 13 14, 11 12 13 14, 11 22 23 24, 11 2 3 4),(1 2 5 6, 11.01 2.01 15 16, 11 2.01 25 26, 1 2 5 6))" ) );
 }
 
 void TestQgsGeometry::triangle()