[convert to curve] PR review

qgis · Jun 18, 2021 · 2c69aca · 2c69aca
1 parent c86f8a8
commit 2c69aca
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 18 deletions.
diff --git a/python/core/auto_generated/geometry/qgsgeometry.sip.in b/python/core/auto_generated/geometry/qgsgeometry.sip.in
@@ -727,7 +727,7 @@ Deletes the vertex at the given position number and item
          object to help make the distinction?)
 %End
 
-    bool convertVertex( int atVertex );
+    bool toggleCircularAtVertex( int atVertex );
 %Docstring
 Converts the vertex at the given position from/to circular
 

diff --git a/src/app/vertextool/qgsvertextool.cpp b/src/app/vertextool/qgsvertextool.cpp
@@ -2576,27 +2576,27 @@ void QgsVertexTool::toggleVertexCurve()
   }
   else
   {
-    // TODO support more than just 1 vertex    
+    // TODO support more than just 1 vertex
     QgisApp::instance()->messageBar()->pushMessage(
       tr( "Could not convert vertex" ),
       tr( "Conversion can only be done on exactly one vertex." ),
       Qgis::Info );
     return;
   }
-  
+
   QgsVectorLayer *layer = toConvert.layer;
 
   if ( ! QgsWkbTypes::isCurvedType( layer->wkbType() ) )
   {
     QgisApp::instance()->messageBar()->pushMessage(
       tr( "Could not convert vertex" ),
-      tr( "Layer of type %1 does not support curved geometries." ).arg( layer->wkbType() ),
+      tr( "Layer of type %1 does not support curved geometries." ).arg( QgsWkbTypes::displayString( layer->wkbType() ) ),
       Qgis::Warning );
     return;
   }
 
   QgsGeometry geom = layer->getFeature( toConvert.fid ).geometry();
-  bool success = geom.convertVertex(toConvert.vertexId);
+  bool success = geom.toggleCircularAtVertex( toConvert.vertexId );
 
   if ( success )
   {
@@ -2609,7 +2609,7 @@ void QgsVertexTool::toggleVertexCurve()
     layer->destroyEditCommand();
     QgisApp::instance()->messageBar()->pushMessage(
       tr( "Could not convert vertex" ),
-      tr( "Start/end of vertices of features and arcs can not be converted." ).arg( layer->wkbType() ),
+      tr( "Start/end of vertices of features and arcs can not be converted." ),
       Qgis::Warning );
   }
 

diff --git a/src/core/geometry/qgscompoundcurve.cpp b/src/core/geometry/qgscompoundcurve.cpp
@@ -928,7 +928,7 @@ bool QgsCompoundCurve::convertVertex( QgsVertexId position )
   // If on CircularString, we need to check if the vertex is a CurveVertex (odd index).
   // If so, we split the subcurve at vertex -1 and +1, , drop the middle part and insert a LineString/CircularString
   // instead with the same points.
-  
+
   // At the end, we call condenseCurves() to merge successible line/circular strings
 
   QVector< QPair<int, QgsVertexId> > curveIds = curveVertexId( position );
@@ -946,7 +946,7 @@ bool QgsCompoundCurve::convertVertex( QgsVertexId position )
     return false;
 
   // TODO factorize circularString and lineString as logic is almost the same
-  if ( const QgsCircularString *circularString = dynamic_cast<const QgsCircularString *>( curve ) )
+  if ( const QgsCircularString *circularString = qgsgeometry_cast<const QgsCircularString *>( curve ) )
   {
     // If it's a circular string, we convert to LineString
 
@@ -970,7 +970,7 @@ bool QgsCompoundCurve::convertVertex( QgsVertexId position )
 
     removeCurve( curveId );
     if ( subVertexId.vertex < points.length() - 2 )
-      mCurves.insert( curveId.release(), curveC.release() );
+      mCurves.insert( curveId, curveC.release() );
     mCurves.insert( curveId, curveB.release() );
     if ( subVertexId.vertex > 1 )
       mCurves.insert( curveId, curveA.release() );

diff --git a/src/core/geometry/qgsgeometry.cpp b/src/core/geometry/qgsgeometry.cpp
@@ -538,7 +538,7 @@ bool QgsGeometry::deleteVertex( int atVertex )
   return d->geometry->deleteVertex( id );
 }
 
-bool QgsGeometry::convertVertex( int atVertex )
+bool QgsGeometry::toggleCircularAtVertex( int atVertex )
 {
 
   if ( !d->geometry )
@@ -554,27 +554,27 @@ bool QgsGeometry::convertVertex( int atVertex )
 
   // If the geom is a collection, we get the concerned part, otherwise, the part is just the whole geom
   QgsAbstractGeometry *part = nullptr;
-  QgsGeometryCollection *owningCollection = dynamic_cast<QgsGeometryCollection *>( geom );
+  QgsGeometryCollection *owningCollection = qgsgeometry_cast<QgsGeometryCollection *>( geom );
   if ( owningCollection != nullptr )
     part = owningCollection->geometryN( id.part );
   else
     part = geom;
 
   // If the part is a polygon, we get the concerned ring, otherwise, the ring is just the whole part
   QgsAbstractGeometry *ring = nullptr;
-  QgsCurvePolygon *owningPolygon = dynamic_cast<QgsCurvePolygon *>( part );
+  QgsCurvePolygon *owningPolygon = qgsgeometry_cast<QgsCurvePolygon *>( part );
   if ( owningPolygon != nullptr )
     ring = ( id.ring == 0 ) ? owningPolygon->exteriorRing() : owningPolygon->interiorRing( id.ring - 1 );
   else
     ring = part;
 
   // If the ring is not a curve, we're probably on a point geometry
-  QgsCurve *curve = dynamic_cast<QgsCurve *>( ring );  // TODO dynamic_cast -> geom_cast
+  QgsCurve *curve = qgsgeometry_cast<QgsCurve *>( ring );
   if ( curve == nullptr )
     return false;
 
   bool success = false;
-  QgsCompoundCurve *cpdCurve  = dynamic_cast<QgsCompoundCurve *>( curve );
+  QgsCompoundCurve *cpdCurve  = qgsgeometry_cast<QgsCompoundCurve *>( curve );
   if ( cpdCurve != nullptr )
   {
     // If the geom is a already compound curve, we convert inplace, and we're done
@@ -603,19 +603,21 @@ bool QgsGeometry::convertVertex( int atVertex )
         // Replace the ring in the owning polygon
         if ( id.ring == 0 )
         {
-          owningPolygon->setExteriorRing( cpdCurve );
+          owningPolygon->setExteriorRing( cpdCurve.get() );
         }
         else
         {
           owningPolygon->removeInteriorRing( id.ring - 1 );
-          owningPolygon->addInteriorRing( cpdCurve );
+          owningPolygon->addInteriorRing( cpdCurve.get() );
+          cpdCurve.release();
         }
       }
       else if ( owningCollection != nullptr )
       {
         // Replace the curve in the owning collection
         owningCollection->removeGeometry( id.part );
-        owningCollection->insertGeometry( cpdCurve, id.part );
+        owningCollection->insertGeometry( cpdCurve.get(), id.part );
+        cpdCurve.release();
       }
     }
   }

diff --git a/src/core/geometry/qgsgeometry.h b/src/core/geometry/qgsgeometry.h
@@ -792,7 +792,7 @@ class CORE_EXPORT QgsGeometry
      * or if the specified vertex can't be converted (e.g. start/end points).
      * \since QGIS 3.20
      */
-    bool convertVertex( int atVertex );
+    bool toggleCircularAtVertex( int atVertex );
 
     /**
      * Returns coordinates of a vertex.