fix mesh refinement crash ( backport #50556)

qgis · Nov 19, 2022 · 2e44db6 · 2e44db6
1 parent 0ff6b2a
commit 2e44db6
Show file tree

Hide file tree

Showing 4 changed files with 126 additions and 39 deletions.
diff --git a/src/app/mesh/qgsmaptooleditmeshframe.cpp b/src/app/mesh/qgsmaptooleditmeshframe.cpp
@@ -640,14 +640,19 @@ bool QgsMapToolEditMeshFrame::populateContextMenuWithEvent( QMenu *menu, QgsMapM
            ( mCurrentFaceIndex != -1 && mCurrentState == Digitizing ) )
       {
         newActions << mActionRemoveFaces;
-        lastActions << mActionFacesRefinement;
       }
 
       if ( mSplittableFaceCount > 0 ||
            ( mCurrentFaceIndex != -1 && mCurrentEditor->faceCanBeSplit( mCurrentFaceIndex ) ) )
         newActions << mActionSplitFaces;
 
-      QList<QAction * > existingActions = menu->actions();
+      int currentFaceSize = mCurrentFaceIndex != -1 ? nativeFace( mCurrentFaceIndex ).size() : 0;
+      if ( mRefinableFaceCount > 0 ||
+           currentFaceSize == 3 ||
+           currentFaceSize == 4 )
+        lastActions << mActionFacesRefinement;
+
+      const QList<QAction * > existingActions = menu->actions();
       if ( !newActions.isEmpty() )
       {
         if ( existingActions.isEmpty() )
@@ -2033,7 +2038,8 @@ void QgsMapToolEditMeshFrame::prepareSelection()
   if ( !mSelectedVertices.isEmpty() )
   {
     double vertexZValue = 0;
-    for ( int i : mSelectedVertices.keys() )
+    const QList selectVertices = mSelectedVertices.keys();
+    for ( int i : selectVertices )
       vertexZValue += mapVertex( i ).z();
     vertexZValue /= mSelectedVertices.count();
 
@@ -2137,10 +2143,10 @@ void QgsMapToolEditMeshFrame::prepareSelection()
     }
   }
 
-  if ( !mSelectedFaces.isEmpty() )
+  const QList<int> facesList = qgis::setToList( mSelectedFaces );
+  if ( !facesList.isEmpty() )
   {
-    const QList<int> faceList = qgis::setToList( mSelectedFaces );
-    QgsGeometry faceGeometrie( new QgsPolygon( new QgsLineString( nativeFaceGeometry( faceList.at( 0 ) ) ) ) );
+    const QgsGeometry faceGeometrie( new QgsPolygon( new QgsLineString( nativeFaceGeometry( facesList.at( 0 ) ) ) ) );
     if ( mSelectedFaces.count() == 1 )
     {
       mSelectedFacesRubberband->setToGeometry( faceGeometrie );
@@ -2151,8 +2157,8 @@ void QgsMapToolEditMeshFrame::prepareSelection()
       geomEngine->prepareGeometry();
 
       QVector<QgsGeometry> otherFaces( mSelectedFaces.count() );
-      for ( int i = 0; i < faceList.count(); ++i )
-        otherFaces[i] = QgsGeometry( new QgsPolygon( new QgsLineString( nativeFaceGeometry( faceList.at( i ) ) ) ) );
+      for ( int i = 0; i < facesList.count(); ++i )
+        otherFaces[i] = QgsGeometry( new QgsPolygon( new QgsLineString( nativeFaceGeometry( facesList.at( i ) ) ) ) );
       QString error;
       const QgsGeometry allFaces( geomEngine->combine( otherFaces, &error ) );
       mSelectedFacesRubberband->setToGeometry( allFaces );
@@ -2179,27 +2185,22 @@ void QgsMapToolEditMeshFrame::prepareSelection()
     mActionRemoveVerticesWithoutFillingHole->setText( tr( "Remove selected vertices without filling hole(s)" ) );
   }
 
-  if ( mSelectedFaces.count() == 1 )
-  {
-    mActionRemoveFaces->setText( tr( "Remove selected face" ) );
-    mActionFacesRefinement->setText( tr( "Refine selected face" ) );
-  }
-  else if ( mSelectedFaces.count() > 1 )
-  {
-    mActionRemoveFaces->setText( tr( "Remove %1 selected faces" ).arg( mSelectedFaces.count() ) );
-    mActionFacesRefinement->setText( tr( "Refine %1 selected faces" ).arg( mSelectedFaces.count() ) );
-  }
+  if ( facesList.count() == 1 )
+    mActionRemoveFaces->setText( tr( "Remove Selected Face" ) );
+  else if ( facesList.count() > 1 )
+    mActionRemoveFaces->setText( tr( "Remove %n Selected Face(s)", nullptr, mSelectedFaces.count() ) );
   else
-  {
-    mActionRemoveFaces->setText( tr( "Remove current face" ) );
-    mActionFacesRefinement->setText( tr( "Refine current face" ) );
-  }
+    mActionRemoveFaces->setText( tr( "Remove Current Face" ) );
 
   mSplittableFaceCount = 0;
+  mRefinableFaceCount = 0;
   for ( const int faceIndex : std::as_const( mSelectedFaces ) )
   {
     if ( mCurrentEditor->faceCanBeSplit( faceIndex ) )
       mSplittableFaceCount++;
+    int faceSize = nativeFace( faceIndex ).size();
+    if ( faceSize == 3 || faceSize == 4 )
+      mRefinableFaceCount++;
   }
 
   if ( mSplittableFaceCount == 1 )
@@ -2209,14 +2210,22 @@ void QgsMapToolEditMeshFrame::prepareSelection()
   else
     mActionSplitFaces->setText( tr( "Split current face" ) );
 
+  if ( mRefinableFaceCount == 1 )
+    mActionFacesRefinement->setText( tr( "Refine Selected Face" ) );
+  else if ( mRefinableFaceCount > 1 )
+    mActionFacesRefinement->setText( tr( "Refine %n Selected Face(s)", nullptr, mRefinableFaceCount ) );
+  else
+    mActionFacesRefinement->setText( tr( "Refine Current Face" ) );
+
   emit selectionChange( mCurrentLayer, mSelectedVertices.keys() );
 }
 
 void QgsMapToolEditMeshFrame::updateSelectecVerticesMarker()
 {
   qDeleteAll( mSelectedVerticesMarker );
   mSelectedVerticesMarker.clear();
-  for ( const int vertexIndex : mSelectedVertices.keys() )
+  const QList selectVertices = mSelectedVertices.keys();
+  for ( const int vertexIndex : selectVertices )
   {
     QgsVertexMarker *marker = new QgsVertexMarker( canvas() );
     marker->setIconType( QgsVertexMarker::ICON_CIRCLE );

diff --git a/src/app/mesh/qgsmaptooleditmeshframe.h b/src/app/mesh/qgsmaptooleditmeshframe.h
@@ -320,6 +320,8 @@ class APP_EXPORT QgsMapToolEditMeshFrame : public QgsMapToolAdvancedDigitizing
 
     //! members for split face
     int mSplittableFaceCount = 0;
+    //! menbers for refinement face
+    int mRefinableFaceCount = 0;
 
     // assiociated widget
     QgsZValueWidget *mZValueWidget = nullptr; //own by QgsUserInputWidget instance

diff --git a/src/core/mesh/qgsmeshadvancedediting.cpp b/src/core/mesh/qgsmeshadvancedediting.cpp
@@ -119,6 +119,15 @@ void QgsMeshEditRefineFaces::createNewVerticesAndRefinedFaces( QgsMeshEditor *me
   int startingVertexIndex = mesh.vertexCount();
   int startingGlobalFaceIndex = mesh.faceCount();
 
+  auto canBeRefined = [ & ]( int fi )->bool
+  {
+    if ( fi < 0 || fi > mesh.faceCount() )
+      return false;
+    int fs = mesh.face( fi ).size();
+    return fs == 3 || fs == 4;
+
+  };
+
   for ( const int faceIndex : std::as_const( mInputFaces ) )
   {
     FaceRefinement refinement;
@@ -128,7 +137,7 @@ void QgsMeshEditRefineFaces::createNewVerticesAndRefinedFaces( QgsMeshEditor *me
 
     QVector<int> addedVerticesIndex( faceSize, -1 );
 
-    if ( faceSize == 3 || faceSize == 4 )
+    if ( canBeRefined( faceIndex ) )
     {
       refinement.newVerticesLocalIndex.reserve( faceSize );
       refinement.refinedFaceNeighbor.reserve( faceSize );
@@ -144,7 +153,7 @@ void QgsMeshEditRefineFaces::createNewVerticesAndRefinedFaces( QgsMeshEditor *me
 
         int neighborFaceIndex = neighbors.at( positionInFace );
         bool needCreateVertex = true;
-        if ( neighborFaceIndex != -1 && facesToRefine.contains( neighborFaceIndex ) )
+        if ( neighborFaceIndex != -1 && facesToRefine.contains( neighborFaceIndex ) && canBeRefined( neighborFaceIndex ) )
         {
           int neighborFaceSize = mesh.face( neighborFaceIndex ).size();
           int positionVertexInNeighbor = vertexPositionInFace( mesh, face.at( positionInFace ), neighborFaceIndex );
@@ -225,26 +234,26 @@ void QgsMeshEditRefineFaces::createNewVerticesAndRefinedFaces( QgsMeshEditor *me
         refinement.newCenterVertexIndex = -1;
 
       facesRefinement.insert( faceIndex, refinement );
+
+      //look for vertexToFace
+      for ( int positionInFace = 0; positionInFace < faceSize; ++positionInFace )
+      {
+        if ( addedVerticesIndex.at( positionInFace ) != -1 )
+        {
+          mVertexToFaceToAdd[addedVerticesIndex.at( positionInFace )] =
+            refinement.newFacesChangesIndex.at( positionInFace ) + startingGlobalFaceIndex;
+        }
+
+        int vertexIndex = face.at( positionInFace );
+        if ( topology.firstFaceLinked( vertexIndex ) == faceIndex )
+          mVerticesToFaceChanges.append( {vertexIndex, faceIndex, refinement.newFacesChangesIndex.at( positionInFace ) + startingGlobalFaceIndex} );
+      }
     }
     else
     {
       //not 3 or 4 vertices, we do not refine this face
       facesToRefine.remove( faceIndex );
     }
-
-    //look for vertexToFace
-    for ( int positionInFace = 0; positionInFace < faceSize; ++positionInFace )
-    {
-      if ( addedVerticesIndex.at( positionInFace ) != -1 )
-      {
-        mVertexToFaceToAdd[addedVerticesIndex.at( positionInFace )] =
-          refinement.newFacesChangesIndex.at( positionInFace ) + startingGlobalFaceIndex;
-      }
-
-      int vertexIndex = face.at( positionInFace );
-      if ( topology.firstFaceLinked( vertexIndex ) == faceIndex )
-        mVerticesToFaceChanges.append( {vertexIndex, faceIndex, refinement.newFacesChangesIndex.at( positionInFace ) + startingGlobalFaceIndex} );
-    }
   }
 
   //all new refined faces are in place, we can build their neighborhood with other new refined faces

diff --git a/tests/src/core/testqgsmesheditor.cpp b/tests/src/core/testqgsmesheditor.cpp
@@ -2021,6 +2021,73 @@ void TestQgsMeshEditor::refineMesh()
         checkRefinedFace( mesh, facesRefinement, i, 2, -1, 3, 4 );
     }
   }
+  {
+    // Attempt to refine a face with 5 vertices (not allowed)
+    QgsMesh mesh;
+    QgsTriangularMesh triangularMesh;
+    QgsMeshEditor meshEditor( &mesh, &triangularMesh );
+    QgsMeshEditingError error;
+
+    mesh.vertices.append( QgsMeshVertex( 0, 200, 0 ) ); // 0
+    mesh.vertices.append( QgsMeshVertex( 200, 200, 0 ) ); // 1
+    mesh.vertices.append( QgsMeshVertex( 0, 0, 0 ) ); // 2
+    mesh.vertices.append( QgsMeshVertex( 200, 0, 0 ) ); //
+    mesh.vertices.append( QgsMeshVertex( 100, 175, 0 ) ); // 4
+    mesh.vertices.append( QgsMeshVertex( 25, 150, 0 ) ); // 5
+    mesh.vertices.append( QgsMeshVertex( 25, 25, 0 ) ); // 6
+    mesh.vertices.append( QgsMeshVertex( 175, 25, 0 ) ); // 7
+    mesh.vertices.append( QgsMeshVertex( 175, 150, 0 ) ); // 8
+
+    mesh.faces.append( {2, 5, 0 } ); // 0
+    mesh.faces.append( {0, 5, 4 } ); // 1
+    mesh.faces.append( {0, 4, 1 } ); // 2
+    mesh.faces.append( {1, 4, 8 } ); // 3
+    mesh.faces.append( {4, 5, 6, 7, 8 } ); // 4
+    mesh.faces.append( {2, 6, 5 } ); // 5
+    mesh.faces.append( {6, 2, 3, 7 } ); // 6
+    mesh.faces.append( {3, 8, 7} ); // 7
+
+    const QgsCoordinateTransform transform;
+    triangularMesh.update( &mesh, transform );
+    QVERIFY( meshEditor.initialize() == QgsMeshEditingError() );
+    QVERIFY( meshEditor.checkConsistency( error ) );
+
+    QCOMPARE( meshEditor.mMesh->faceCount(), 8 );
+    QCOMPARE( meshEditor.mMesh->vertexCount(), 9 );
+
+    QList<int> facesList;
+    facesList << 4;
+
+    QgsMeshEditRefineFaces refineEditing;
+    refineEditing.setInputFaces( facesList );
+    QHash<int, QgsMeshEditRefineFaces::FaceRefinement > facesRefinement;
+    QHash<int, QgsMeshEditRefineFaces::BorderFace> borderFaces;
+    QSet<int> facesToRefine;
+    facesToRefine = qgis::listToSet( facesList );
+
+    refineEditing.createNewVerticesAndRefinedFaces( &meshEditor, facesToRefine, facesRefinement );
+    refineEditing.createNewBorderFaces( &meshEditor, facesToRefine, facesRefinement, borderFaces );
+
+    // refinement not done
+    QVERIFY( facesRefinement.isEmpty() );
+    QVERIFY( borderFaces.isEmpty() );
+
+    facesList.clear();
+    facesList << 0 << 1 << 2 << 3 << 4 << 5 << 6 << 7;
+    refineEditing = QgsMeshEditRefineFaces();
+    refineEditing.setInputFaces( facesList );
+    facesRefinement.clear();
+    borderFaces.clear();
+    facesToRefine.clear();
+    facesToRefine = qgis::listToSet( facesList );
+
+    refineEditing.createNewVerticesAndRefinedFaces( &meshEditor, facesToRefine, facesRefinement );
+    refineEditing.createNewBorderFaces( &meshEditor, facesToRefine, facesRefinement, borderFaces );
+
+    QVERIFY( !facesRefinement.isEmpty() );
+    QVERIFY( !borderFaces.isEmpty() );
+
+  }
 }
 
 void TestQgsMeshEditor::transformByExpression()