Validate polygon split geometry before committing changes (#4211)

Author: VitorVieiraZ

app/maptools/splittingmaptool.cpp

@@ -42,14 +42,40 @@ bool SplittingMapTool::hasValidGeometry() const
   return mPoints.count() >= 2;
 }
 
-bool SplittingMapTool::commitSplit() const
+bool SplittingMapTool::isValidSplit() const
 {
-  if ( !mFeatureToSplit.isValid() )
+  if ( !mFeatureToSplit.isValid() || !hasValidGeometry() )
     return false;
 
-  if ( !hasValidGeometry() )
+  const QgsGeometry featureGeom = mFeatureToSplit.feature().geometry();
+  if ( featureGeom.isNull() || featureGeom.isEmpty() )
+    return false;
+
+  // split line must intersect the feature's geometry
+  if ( !mRecordedGeometry.intersects( featureGeom ) )
     return false;
 
+  QgsPointXY firstPoint( mPoints.first() );
+  QgsPointXY lastPoint( mPoints.last() );
+
+  // start and end points of the line must be outside the feature, ensuring line properly crosses the feature
+  if ( featureGeom.contains( &firstPoint ) || featureGeom.contains( &lastPoint ) )
+    return false;
+
+  return true;
+}
+
+SplittingMapTool::SplitResult SplittingMapTool::commitSplit() const
+{
+  if ( !mFeatureToSplit.isValid() )
+    return Failed;
+
+  if ( !hasValidGeometry() )
+    return Failed;
+
+  if ( !isValidSplit() )
+    return InvalidSplit;
+
   // only the specified featureToSplit shall be split, so we select
   // it here in order to avoid other features being split
   mFeatureToSplit.layer()->select( mFeatureToSplit.feature().id() );
@@ -62,9 +88,9 @@ bool SplittingMapTool::commitSplit() const
 
   if ( result == Qgis::GeometryOperationResult::Success )
   {
-    return true;
+    return Success;
   }
-  return false;
+  return Failed;
 }
 
 void SplittingMapTool::rebuildGeometry()

app/maptools/splittingmaptool.h

@@ -25,6 +25,15 @@ class SplittingMapTool : public AbstractMapTool
     Q_PROPERTY( FeatureLayerPair featureToSplit READ featureToSplit WRITE setFeatureToSplit NOTIFY featureToSplitChanged )
 
   public:
+
+    enum SplitResult
+    {
+      Success = 0,
+      InvalidSplit = 1, // Split line does not properly cross the feature boundary
+      Failed = 2
+    };
+    Q_ENUM( SplitResult )
+
     explicit SplittingMapTool( QObject *parent = nullptr );
     virtual ~SplittingMapTool() override;
 
@@ -43,11 +52,8 @@ class SplittingMapTool : public AbstractMapTool
     //! Returns true if the captured geometry has enought points for the specified layer
     Q_INVOKABLE bool hasValidGeometry() const;
 
-    /**
-     * Splits the feature with recorded geometry and commits the changes to the layer
-     * Returns true if splitting was successful, false otherwise
-     */
-    Q_INVOKABLE bool commitSplit() const;
+    //! Splits the feature with recorded geometry and commits the changes to the layer
+    Q_INVOKABLE SplittingMapTool::SplitResult commitSplit() const;
 
     // Getters/setters
     const QgsGeometry &recordedGeometry() const;
@@ -64,6 +70,12 @@ class SplittingMapTool : public AbstractMapTool
     void rebuildGeometry();
 
   private:
+    /**
+     * Validates if the recorded geometry can successfully split the feature
+     * Returns true if the split line properly crosses the feature boundary
+     */
+    bool isValidSplit() const;
+
     QVector<QgsPoint> mPoints;
 
     QgsGeometry mRecordedGeometry;

app/qml/dialogs/MMSplittingFailedDialog.qml

@@ -16,7 +16,7 @@ MMDrawerDialog {
 
   imageSource: __style.negativeMMSymbolImage
   title: qsTr( "We could not split the feature" )
-  description: qsTr( "Please make sure that the split line crosses your feature. The feature needs to have a valid geometry in order to split it." )
+  description: qsTr( "Ensure that your split line intersects the feature properly and that the feature’s geometry is valid." )
   primaryButton.text: qsTr( "Ok, I understand" )
 
   onPrimaryButtonClicked: close()

app/qml/map/MMSplittingTools.qml

@@ -106,7 +106,14 @@ Item {
           if ( mapTool.hasValidGeometry() )
           {
             let result = mapTool.commitSplit()
-            root.done( result )
+
+            if ( result === MM.SplittingMapTool.InvalidSplit )
+            {
+              __notificationModel.addWarning( qsTr( "The split line does not properly cross the feature. Please adjust the line to cross the feature boundary." ) )
+              return
+            }
+
+            root.done( result === MM.SplittingMapTool.Success )
           }
           else
           {

app/test/testmaptools.cpp

@@ -195,6 +195,74 @@ void TestMapTools::testSplitting()
   delete splitTool;
 }
 
+void TestMapTools::testValidSplit()
+{
+  // splitting environment setup
+  SplittingMapTool *splitTool = new SplittingMapTool();
+
+  QString projectDir = TestUtils::testDataDir() + "/planes";
+  QString projectTempDir = QDir::tempPath() + "/" + QUuid::createUuid().toString();
+  QString projectName = "quickapp_project.qgs";
+
+  QVERIFY( InputUtils::cpDir( projectDir, projectTempDir ) );
+
+  QgsProject *project = new QgsProject();
+
+  QVERIFY( project->read( projectTempDir + "/" + projectName ) );
+
+  QgsMapLayer *sectorL = project->mapLayersByName( QStringLiteral( "FlySector" ) ).at( 0 );
+  QgsVectorLayer *flySectorLayer = static_cast<QgsVectorLayer *>( sectorL );
+
+  QVERIFY( flySectorLayer && flySectorLayer->isValid() );
+
+  InputMapCanvasMap canvas;
+  InputMapSettings *ms = canvas.mapSettings();
+  ms->setProject( project );
+  ms->setDestinationCrs( project->crs() );
+  ms->setOutputSize( QSize( 600, 1096 ) );
+  ms->setLayers( project->layers<QgsMapLayer *>().toList() );
+
+  QgsRectangle extent = QgsRectangle( -107.54331499504026226, 21.62302175066136556, -72.73224633912816728, 51.49933451998575151 );
+  ms->setExtent( extent );
+
+  splitTool->setMapSettings( ms );
+
+  // set feature to split
+  int fidToSplit = 1;
+  QgsFeature featureToSplit = flySectorLayer->getFeature( fidToSplit );
+  FeatureLayerPair pairToSplit( featureToSplit, flySectorLayer );
+  splitTool->setFeatureToSplit( pairToSplit );
+
+  // not enough points
+  QCOMPARE( splitTool->commitSplit(), SplittingMapTool::Failed );
+
+  // line doesnt intersect feature
+  splitTool->addPoint( QgsPoint( -104.751, 32.448 ) );
+  QCOMPARE( splitTool->commitSplit(), SplittingMapTool::Failed );
+
+  // valid split line, endpoints outside and line intersects feature
+  splitTool->addPoint( QgsPoint( -120.844, 32.592 ) );
+  QCOMPARE( splitTool->commitSplit(), SplittingMapTool::Success );
+
+  // line doesnt intersect feature
+  splitTool->removePoint();
+  splitTool->removePoint();
+  splitTool->addPoint( QgsPoint( -130.0, 10.0 ) );
+  splitTool->addPoint( QgsPoint( -140.0, 15.0 ) );
+  QCOMPARE( splitTool->commitSplit(), SplittingMapTool::InvalidSplit );
+
+  // endpoint inside feature boundary
+  QgsPointXY centerPoint = featureToSplit.geometry().centroid().asPoint();
+  splitTool->removePoint();
+  splitTool->removePoint();
+  splitTool->addPoint( QgsPoint( centerPoint.x(), centerPoint.y() ) );
+  splitTool->addPoint( QgsPoint( -120.844, 32.592 ) );
+  QCOMPARE( splitTool->commitSplit(), SplittingMapTool::InvalidSplit );
+
+  delete project;
+  delete splitTool;
+}
+
 void TestMapTools::testRecording()
 {
   RecordingMapTool *recordTool = new RecordingMapTool();

app/test/testmaptools.h

@@ -26,7 +26,10 @@ class TestMapTools : public QObject
     void cleanup();
 
     void testSnapping();
+
     void testSplitting();
+    void testValidSplit();
+
     void testRecording();
     void testMeasuring();