Merge pull request #2298 from nyalldawson/fix_13099

Fix #13099 - removal of geometries using node tool

python/core/geometry/qgsgeometrycollectionv2.sip

@@ -21,6 +21,13 @@ class QgsGeometryCollectionV2: public QgsAbstractGeometryV2
 
     /** Adds a geometry and takes ownership. Returns true in case of success*/
     virtual bool addGeometry( QgsAbstractGeometryV2* g /Transfer/ );
+
+    /** Inserts a geometry before a specified index and takes ownership. Returns true in case of success.
+     * @param g geometry to insert. Ownership is transferred to the collection.
+     * @param index position to insert geometry before
+    */
+    virtual bool insertGeometry( QgsAbstractGeometryV2* g /Transfer/, int index );
+
     virtual bool removeGeometry( int nr );
 
     virtual void transform( const QgsCoordinateTransform& ct, QgsCoordinateTransform::TransformDirection d = QgsCoordinateTransform::ForwardTransform );

src/core/geometry/qgscurvepolygonv2.cpp

@@ -447,7 +447,7 @@ bool QgsCurvePolygonV2::removeInteriorRing( int nr )
   {
     return false;
   }
-  mInteriorRings.removeAt( nr );
+  delete mInteriorRings.takeAt( nr );
   return true;
 }
 
@@ -626,10 +626,26 @@ bool QgsCurvePolygonV2::deleteVertex( const QgsVertexId& vId )
 
   QgsCurveV2* ring = vId.ring == 0 ? mExteriorRing : mInteriorRings[vId.ring - 1];
   int n = ring->numPoints();
-  if ( n <= 4 )
+  if ( n <= 2 )
   {
-    return false;
+    //no points will be left in ring, so remove whole ring
+    if ( vId.ring == 0 )
+    {
+      delete mExteriorRing;
+      mExteriorRing = 0;
+      if ( !mInteriorRings.isEmpty() )
+      {
+        mExteriorRing = mInteriorRings.takeFirst();
+      }
+    }
+    else
+    {
+      removeInteriorRing( vId.ring - 1 );
+    }
+    mBoundingBox = QgsRectangle();
+    return true;
   }
+
   bool success = ring->deleteVertex( vId );
   if ( success )
   {

src/core/geometry/qgsgeometry.cpp

@@ -438,6 +438,15 @@ bool QgsGeometry::deleteVertex( int atVertex )
     return false;
   }
 
+  //maintain compatibility with < 2.10 API
+  if ( d->geometry->geometryType() == "MultiPoint" )
+  {
+    detach( true );
+    removeWkbGeos();
+    //delete geometry instead of point
+    return static_cast< QgsGeometryCollectionV2* >( d->geometry )->removeGeometry( atVertex );
+  }
+
   //if it is a point, set the geometry to NULL
   if ( QgsWKBTypes::flatType( d->geometry->wkbType() ) == QgsWKBTypes::Point )
   {
@@ -467,15 +476,25 @@ bool QgsGeometry::insertVertex( double x, double y, int beforeVertex )
     return false;
   }
 
-  detach( true );
+  //maintain compatibility with < 2.10 API
+  if ( d->geometry->geometryType() == "MultiPoint" )
+  {
+    detach( true );
+    removeWkbGeos();
+    //insert geometry instead of point
+    return static_cast< QgsGeometryCollectionV2* >( d->geometry )->insertGeometry( new QgsPointV2( x, y ), beforeVertex );
+  }
 
   QgsVertexId id;
   if ( !vertexIdFromVertexNr( beforeVertex, id ) )
   {
     return false;
   }
 
+  detach( true );
+
   removeWkbGeos();
+
   return d->geometry->insertVertex( id, QgsPointV2( x, y ) );
 }
 

src/core/geometry/qgsgeometrycollectionv2.cpp

@@ -105,9 +105,20 @@ bool QgsGeometryCollectionV2::addGeometry( QgsAbstractGeometryV2* g )
   return true;
 }
 
+bool QgsGeometryCollectionV2::insertGeometry( QgsAbstractGeometryV2 *g, int index )
+{
+  if ( !g )
+  {
+    return false;
+  }
+
+  mGeometries.insert( index, g );
+  return true;
+}
+
 bool QgsGeometryCollectionV2::removeGeometry( int nr )
 {
-  if ( nr >= mGeometries.size() )
+  if ( nr >= mGeometries.size() || nr < 0 )
   {
     return false;
   }

src/core/geometry/qgsgeometrycollectionv2.h

@@ -57,6 +57,12 @@ class CORE_EXPORT QgsGeometryCollectionV2: public QgsAbstractGeometryV2
     /** Adds a geometry and takes ownership. Returns true in case of success.*/
     virtual bool addGeometry( QgsAbstractGeometryV2* g );
 
+    /** Inserts a geometry before a specified index and takes ownership. Returns true in case of success.
+     * @param g geometry to insert. Ownership is transferred to the collection.
+     * @param index position to insert geometry before
+    */
+    virtual bool insertGeometry( QgsAbstractGeometryV2* g, int index );
+
     /** Removes a geometry from the collection.
      * @param nr index of geometry to remove
      * @returns true if removal was successful.

tests/src/python/test_qgsgeometry.py

@@ -433,7 +433,7 @@ def testClosestVertex(self):
         (dist, minDistPoint, afterVertex) = polygon.closestSegmentWithContext(QgsPoint(0.7, 1.1))
         self.assertEqual(afterVertex, 2)
         self.assertEqual(minDistPoint, QgsPoint(1, 1))
-        exp = 0.3**2 + 0.1**2
+        exp = 0.3 ** 2 + 0.1 ** 2
         assert abs(dist - exp) < 0.00001, "Expected: %f; Got:%f" % (exp, dist)
 
         # 3-+-+-2
@@ -747,6 +747,42 @@ def testMultipoint(self):
             assert p == points[i], "Expected %s at %d, got %s" % (points[i].toString(), i, p.toString())
             i += 1
 
+        multipoint = QgsGeometry.fromWkt("MultiPoint ((5 5))")
+        assert multipoint.vertexAt(0) == QgsPoint(5, 5), "MULTIPOINT fromWkt failed"
+
+        assert multipoint.insertVertex(4, 4, 0), "MULTIPOINT insert 4,4 at 0 failed"
+        expwkt = "MultiPoint ((4 4),(5 5))"
+        wkt = multipoint.exportToWkt()
+        assert compareWkt(expwkt, wkt), "Expected:\n%s\nGot:\n%s\n" % (expwkt, wkt)
+
+        assert multipoint.insertVertex(7, 7, 2), "MULTIPOINT append 7,7 at 2 failed"
+        expwkt = "MultiPoint ((4 4),(5 5),(7 7))"
+        wkt = multipoint.exportToWkt()
+        assert compareWkt(expwkt, wkt), "Expected:\n%s\nGot:\n%s\n" % (expwkt, wkt)
+
+        assert multipoint.insertVertex(6, 6, 2), "MULTIPOINT append 6,6 at 2 failed"
+        expwkt = "MultiPoint ((4 4),(5 5),(6 6),(7 7))"
+        wkt = multipoint.exportToWkt()
+        assert compareWkt(expwkt, wkt), "Expected:\n%s\nGot:\n%s\n" % (expwkt, wkt)
+
+        assert not multipoint.deleteVertex(4), "MULTIPOINT delete at 4 unexpectedly succeeded"
+        assert not multipoint.deleteVertex(-1), "MULTIPOINT delete at -1 unexpectedly succeeded"
+
+        assert multipoint.deleteVertex(1), "MULTIPOINT delete at 1 failed"
+        expwkt = "MultiPoint ((4 4),(6 6),(7 7))"
+        wkt = multipoint.exportToWkt()
+        assert compareWkt(expwkt, wkt), "Expected:\n%s\nGot:\n%s\n" % (expwkt, wkt)
+
+        assert multipoint.deleteVertex(2), "MULTIPOINT delete at 2 failed"
+        expwkt = "MultiPoint ((4 4),(6 6))"
+        wkt = multipoint.exportToWkt()
+        assert compareWkt(expwkt, wkt), "Expected:\n%s\nGot:\n%s\n" % (expwkt, wkt)
+
+        assert multipoint.deleteVertex(0), "MULTIPOINT delete at 2 failed"
+        expwkt = "MultiPoint ((6 6))"
+        wkt = multipoint.exportToWkt()
+        assert compareWkt(expwkt, wkt), "Expected:\n%s\nGot:\n%s\n" % (expwkt, wkt)
+
         multipoint = QgsGeometry.fromWkt("MultiPoint ((5 5))")
         assert multipoint.vertexAt(0) == QgsPoint(5, 5), "MultiPoint fromWkt failed"
 
@@ -928,10 +964,10 @@ def testDeleteVertex(self):
         assert compareWkt(expwkt, wkt), "Expected:\n%s\nGot:\n%s\n" % (expwkt, wkt)
 
         polygon = QgsGeometry.fromWkt("MultiPolygon (((0 0, 1 0, 1 1, 2 1, 2 2, 0 2, 0 0)),((4 0, 5 0, 5 2, 3 2, 3 1, 4 1, 4 0)))")
-        for i in range(3):  # cannot have less than four points in a ring
+        for i in range(6):
             assert polygon.deleteVertex(0), "Delete vertex 0 failed"
 
-        expwkt = "MultiPolygon (((2 1, 2 2, 0 2, 2 1)),((4 0, 5 0, 5 2, 3 2, 3 1, 4 1, 4 0)))"
+        expwkt = "MultiPolygon (((4 0, 5 0, 5 2, 3 2, 3 1, 4 1, 4 0)))"
         wkt = polygon.exportToWkt()
         assert compareWkt(expwkt, wkt), "Expected:\n%s\nGot:\n%s\n" % (expwkt, wkt)
 
@@ -943,19 +979,29 @@ def testDeleteVertex(self):
         # |                 |
         # 0-+-+-+-+---+-+-+-1
         polygon = QgsGeometry.fromWkt("Polygon ((0 0, 9 0, 9 3, 0 3, 0 0),(1 1, 2 1, 2 2, 1 2, 1 1),(3 1, 4 1, 4 2, 3 2, 3 1),(5 1, 6 1, 6 2, 5 2, 5 1),(7 1, 8 1, 8 2, 7 2, 7 1))")
-        #                                          0   1   2   3   4     5   6   7   8   9    10  11  12  13  14    15  16  17  18  19    20  21  22  23  24
+        #                                         0   1    2    3    4     5    6    7    8    9     10   11   12   13   14    15   16   17   18   19    20  21   22   23   24
+
+        for i in range(4):
+            assert polygon.deleteVertex(16), "Delete vertex 16 failed" % i
+
+        expwkt = "Polygon ((0 0, 9 0, 9 3, 0 3, 0 0),(1 1, 2 1, 2 2, 1 2, 1 1),(3 1, 4 1, 4 2, 3 2, 3 1),(7 1, 8 1, 8 2, 7 2, 7 1))"
+        wkt = polygon.exportToWkt()
+        assert compareWkt(expwkt, wkt), "Expected:\n%s\nGot:\n%s\n" % (expwkt, wkt)
 
-        #cannot have less than 4 vertices in a ring
-        assert polygon.deleteVertex(16), "Delete vertex 16 failed" % i
+        for i in range(3):
+            for j in range(4):
+                assert polygon.deleteVertex(5), "Delete vertex 5 failed" % i
 
-        expwkt = "Polygon ((0 0, 9 0, 9 3, 0 3, 0 0),(1 1, 2 1, 2 2, 1 2, 1 1),(3 1, 4 1, 4 2, 3 2, 3 1),(5 1, 6 2, 5 2, 5 1),(7 1, 8 1, 8 2, 7 2, 7 1))"
+        expwkt = "Polygon ((0 0, 9 0, 9 3, 0 3, 0 0))"
         wkt = polygon.exportToWkt()
         assert compareWkt(expwkt, wkt), "Expected:\n%s\nGot:\n%s\n" % (expwkt, wkt)
 
-        #ring needs to have at least 4 vertices!
-        assert polygon.deleteVertex(5), "Delete vertex 5 failed" % i
+        #Remove whole outer ring, inner ring should become outer
+        polygon = QgsGeometry.fromWkt("Polygon ((0 0, 9 0, 9 3, 0 3, 0 0),(1 1, 2 1, 2 2, 1 2, 1 1))")
+        for i in range(4):
+            assert polygon.deleteVertex(0), "Delete vertex 16 failed" % i
 
-        expwkt = "Polygon ((0 0, 9 0, 9 3, 0 3, 0 0),(2 1, 2 2, 1 2, 2 1),(3 1, 4 1, 4 2, 3 2, 3 1),(5 1, 6 2, 5 2, 5 1),(7 1, 8 1, 8 2, 7 2, 7 1))"
+        expwkt = "Polygon ((1 1, 2 1, 2 2, 1 2, 1 1))"
         wkt = polygon.exportToWkt()
         assert compareWkt(expwkt, wkt), "Expected:\n%s\nGot:\n%s\n" % (expwkt, wkt)