Fix inconsistent behaviour when deleting nodes with node tool

(fix #14168)
qgis · Jan 27, 2016 · 4ec0912 · 4ec0912
1 parent 47013f7
commit 4ec0912
Show file tree

Hide file tree

Showing 7 changed files with 101 additions and 20 deletions.
diff --git a/python/core/qgsvectorlayer.sip b/python/core/qgsvectorlayer.sip
@@ -312,6 +312,16 @@ class QgsVectorLayer : QgsMapLayer
       Color,          /**< color */
       EditorWidgetV2, /**< modularized edit widgets @note added in 2.1 */
     };
+
+    //! Result of an edit operation
+    enum EditResult
+    {
+      Success, /**< Edit operation was successful */
+      EmptyGeometry, /**< Edit operation resulted in an empty geometry */
+      EditFailed, /**< Edit operation failed */
+      FetchFeatureFailed, /**< Unable to fetch requested feature */
+      InvalidLayer, /**< Edit failed due to invalid layer */
+    };
 
     /** Types of feature form suppression after feature creation
      * @note added in 2.1 */
@@ -793,8 +803,17 @@ class QgsVectorLayer : QgsMapLayer
     bool moveVertex( const QgsPointV2& p, QgsFeatureId atFeatureId, int atVertex ) /PyName=moveVertexV2/;
 
     /** Deletes a vertex from a feature
+     * @deprecated use deleteVertexV2() instead
+     */
+    bool deleteVertex( QgsFeatureId atFeatureId, int atVertex ) /Deprecated/;
+
+    /** Deletes a vertex from a feature.
+     * @param featureId ID of feature to remove vertex from
+     * @param vertex index of vertex to delete
+     * @note added in QGIS 2.14
      */
-    bool deleteVertex( QgsFeatureId atFeatureId, int atVertex );
+    //TODO QGIS 3.0 - rename back to deleteVertex
+    EditResult deleteVertexV2( QgsFeatureId featureId, int vertex );
 
     /** Deletes the selected features
      *  @return true in case of success and false otherwise

diff --git a/python/core/qgsvectorlayereditutils.sip b/python/core/qgsvectorlayereditutils.sip
@@ -22,8 +22,17 @@ class QgsVectorLayerEditUtils
     bool moveVertex( double x, double y, QgsFeatureId atFeatureId, int atVertex );
 
     /** Deletes a vertex from a feature
+     * @deprecated use deleteVertexV2() instead
      */
-    bool deleteVertex( QgsFeatureId atFeatureId, int atVertex );
+    bool deleteVertex( QgsFeatureId atFeatureId, int atVertex ) /Deprecated/;
+
+    /** Deletes a vertex from a feature.
+     * @param featureId ID of feature to remove vertex from
+     * @param vertex index of vertex to delete
+     * @node added in QGIS 2.14
+     */
+    //TODO QGIS 3.0 - rename to deleteVertex
+    QgsVectorLayer::EditResult deleteVertexV2( QgsFeatureId featureId, int vertex );
 
     /** Adds a ring to polygon/multipolygon features
      * @param ring ring to add

diff --git a/src/app/nodetool/qgsselectedfeature.cpp b/src/app/nodetool/qgsselectedfeature.cpp
@@ -246,7 +246,8 @@ void QgsSelectedFeature::deleteSelectedVertexes()
 
   beginGeometryChange();
 
-  int count = 0;
+  bool success = false;
+  QgsVectorLayer::EditResult res = QgsVectorLayer::Success;
   for ( int i = mVertexMap.size() - 1; i > -1 && nSelected > 0; i-- )
   {
     if ( mVertexMap.at( i )->isSelected() )
@@ -263,14 +264,17 @@ void QgsSelectedFeature::deleteSelectedVertexes()
       if ( --nSelected == 0 )
         endGeometryChange();
 
-      if ( !mVlayer->deleteVertex( mFeatureId, i ) )
+      if ( res != QgsVectorLayer::EmptyGeometry )
+        res = mVlayer->deleteVertexV2( mFeatureId, i );
+
+      if ( res != QgsVectorLayer::Success && res != QgsVectorLayer::EmptyGeometry )
       {
-        count = 0;
+        success = false;
         QgsDebugMsg( QString( "Deleting vertex %1 failed - resetting" ).arg( i ) );
         break;
       }
 
-      count++;
+      success = true;
 
       if ( topologicalEditing )
       {
@@ -283,13 +287,22 @@ void QgsSelectedFeature::deleteSelectedVertexes()
             mVlayer->deleteVertex( resultIt.value().snappedAtGeometry, resultIt.value().snappedVertexNr );
         }
       }
+
+      if ( res == QgsVectorLayer::EmptyGeometry )
+      {
+        //geometry has been cleared as a result of deleting vertices (eg not enough vertices left to leave a valid geometry),
+        //so nothing more to do
+        QgsGeometry empty;
+        geometryChanged( mFeatureId, empty );
+        break;
+      }
     }
   }
 
   if ( nSelected > 0 )
     endGeometryChange();
 
-  if ( count > 0 )
+  if ( success )
   {
     mVlayer->endEditCommand();
   }

diff --git a/src/core/qgsvectorlayer.cpp b/src/core/qgsvectorlayer.cpp
@@ -1025,12 +1025,18 @@ bool QgsVectorLayer::moveVertex( const QgsPointV2& p, QgsFeatureId atFeatureId,
 }
 
 bool QgsVectorLayer::deleteVertex( QgsFeatureId atFeatureId, int atVertex )
+{
+  QgsVectorLayer::EditResult res = deleteVertexV2( atFeatureId, atVertex );
+  return res == QgsVectorLayer::Success || res == QgsVectorLayer::EmptyGeometry;
+}
+
+QgsVectorLayer::EditResult QgsVectorLayer::deleteVertexV2( QgsFeatureId featureId, int vertex )
 {
   if ( !mValid || !mEditBuffer || !mDataProvider )
-    return false;
+    return QgsVectorLayer::InvalidLayer;
 
   QgsVectorLayerEditUtils utils( this );
-  return utils.deleteVertex( atFeatureId, atVertex );
+  return utils.deleteVertexV2( featureId, vertex );
 }
 
 

diff --git a/src/core/qgsvectorlayer.h b/src/core/qgsvectorlayer.h
@@ -475,6 +475,16 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer
       EditorWidgetV2, /**< modularized edit widgets @note added in 2.1 */
     };
 
+    //! Result of an edit operation
+    enum EditResult
+    {
+      Success = 0, /**< Edit operation was successful */
+      EmptyGeometry = 1, /**< Edit operation resulted in an empty geometry */
+      EditFailed = 2, /**< Edit operation failed */
+      FetchFeatureFailed = 3, /**< Unable to fetch requested feature */
+      InvalidLayer = 4, /**< Edit failed due to invalid layer */
+    };
+
     /** Constructor - creates a vector layer
      *
      * The QgsVectorLayer is constructed by instantiating a data provider.  The provider
@@ -908,8 +918,17 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer
     bool moveVertex( const QgsPointV2& p, QgsFeatureId atFeatureId, int atVertex );
 
     /** Deletes a vertex from a feature
+     * @deprecated use deleteVertexV2() instead
+     */
+    Q_DECL_DEPRECATED bool deleteVertex( QgsFeatureId atFeatureId, int atVertex );
+
+    /** Deletes a vertex from a feature.
+     * @param featureId ID of feature to remove vertex from
+     * @param vertex index of vertex to delete
+     * @note added in QGIS 2.14
      */
-    bool deleteVertex( QgsFeatureId atFeatureId, int atVertex );
+    //TODO QGIS 3.0 - rename back to deleteVertex
+    EditResult deleteVertexV2( QgsFeatureId featureId, int vertex );
 
     /** Deletes the selected features
      *  @return true in case of success and false otherwise

diff --git a/src/core/qgsvectorlayereditutils.cpp b/src/core/qgsvectorlayereditutils.cpp
@@ -85,33 +85,38 @@ bool QgsVectorLayerEditUtils::moveVertex( const QgsPointV2& p, QgsFeatureId atFe
 
 
 bool QgsVectorLayerEditUtils::deleteVertex( QgsFeatureId atFeatureId, int atVertex )
+{
+  QgsVectorLayer::EditResult res = deleteVertexV2( atFeatureId, atVertex );
+  return res == QgsVectorLayer::Success || res == QgsVectorLayer::EmptyGeometry;
+}
+
+QgsVectorLayer::EditResult QgsVectorLayerEditUtils::deleteVertexV2( QgsFeatureId featureId, int vertex )
 {
   if ( !L->hasGeometryType() )
-    return false;
+    return QgsVectorLayer::InvalidLayer;
 
   QgsGeometry geometry;
-  if ( !cache()->geometry( atFeatureId, geometry ) )
+  if ( !cache()->geometry( featureId, geometry ) )
   {
     // it's not in cache: let's fetch it from layer
     QgsFeature f;
-    if ( !L->getFeatures( QgsFeatureRequest().setFilterFid( atFeatureId ).setSubsetOfAttributes( QgsAttributeList() ) ).nextFeature( f ) || !f.constGeometry() )
-      return false; // geometry not found
+    if ( !L->getFeatures( QgsFeatureRequest().setFilterFid( featureId ).setSubsetOfAttributes( QgsAttributeList() ) ).nextFeature( f ) || !f.constGeometry() )
+      return QgsVectorLayer::FetchFeatureFailed; // geometry not found
 
     geometry = *f.constGeometry();
   }
 
-  if ( !geometry.deleteVertex( atVertex ) )
-    return false;
+  if ( !geometry.deleteVertex( vertex ) )
+    return QgsVectorLayer::EditFailed;
 
   if ( geometry.geometry() && geometry.geometry()->nCoordinates() == 0 )
   {
     //last vertex deleted, set geometry to null
     geometry.setGeometry( nullptr );
   }
 
-
-  L->editBuffer()->changeGeometry( atFeatureId, &geometry );
-  return true;
+  L->editBuffer()->changeGeometry( featureId, &geometry );
+  return !geometry.isEmpty() ? QgsVectorLayer::Success : QgsVectorLayer::EmptyGeometry;
 }
 
 int QgsVectorLayerEditUtils::addRing( const QList<QgsPoint>& ring, const QgsFeatureIds& targetFeatureIds, QgsFeatureId* modifiedFeatureId )

diff --git a/src/core/qgsvectorlayereditutils.h b/src/core/qgsvectorlayereditutils.h
@@ -26,6 +26,7 @@ class QgsCurveV2;
 class CORE_EXPORT QgsVectorLayerEditUtils
 {
   public:
+
     QgsVectorLayerEditUtils( QgsVectorLayer* layer );
 
     inline QgsGeometryCache* cache() { return L->cache(); }
@@ -50,8 +51,17 @@ class CORE_EXPORT QgsVectorLayerEditUtils
     bool moveVertex( const QgsPointV2& p, QgsFeatureId atFeatureId, int atVertex );
 
     /** Deletes a vertex from a feature
+     * @deprecated use deleteVertexV2() instead
+     */
+    Q_DECL_DEPRECATED bool deleteVertex( QgsFeatureId atFeatureId, int atVertex );
+
+    /** Deletes a vertex from a feature.
+     * @param featureId ID of feature to remove vertex from
+     * @param vertex index of vertex to delete
+     * @node added in QGIS 2.14
      */
-    bool deleteVertex( QgsFeatureId atFeatureId, int atVertex );
+    //TODO QGIS 3.0 - rename to deleteVertex
+    QgsVectorLayer::EditResult deleteVertexV2( QgsFeatureId featureId, int vertex );
 
     /** Adds a ring to polygon/multipolygon features
      * @param ring ring to add