Fix circular string digitizing tool (#51943)

Fix QgsMapToolShapeCircularStringAbstract when layer crs != map crs
qgis · Apr 6, 2023 · 5f48de9 · 5f48de9
1 parent 80ab370
commit 5f48de9
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 11 deletions.
diff --git a/src/app/maptools/qgsmaptoolshapecircularstringabstract.cpp b/src/app/maptools/qgsmaptoolshapecircularstringabstract.cpp
@@ -20,8 +20,9 @@
 #include "qgsgeometryrubberband.h"
 #include "qgsgeometryutils.h"
 #include "qgslinestring.h"
-#include "qgspoint.h"
+#include "qgsmapcanvas.h"
 #include "qgsmaptoolcapture.h"
+#include "qgspoint.h"
 
 QgsMapToolShapeCircularStringAbstract::QgsMapToolShapeCircularStringAbstract( const QString &id, QgsMapToolCapture *parentTool )
   : QgsMapToolShapeAbstract( id, parentTool )
@@ -189,7 +190,27 @@ void QgsMapToolShapeCircularStringAbstract::addCurveToParentTool()
 {
   QgsCircularString *c = new QgsCircularString();
   c->setPoints( mPoints );
-  mParentTool->addCurve( c );
+
+  // Check whether to draw the circle as a polygon or a circular string
+  bool drawAsPolygon = false;
+
+  if ( QgsMapLayer *layer = mParentTool->layer() )
+  {
+    const QgsCoordinateReferenceSystem layerCrs = layer->crs();
+    const QgsCoordinateReferenceSystem mapCrs = mParentTool->canvas()->mapSettings().destinationCrs();
+    drawAsPolygon = layerCrs != mapCrs;
+  }
+
+  if ( drawAsPolygon )
+  {
+    std::unique_ptr<QgsLineString> ls( c->curveToLine( ) );
+    mParentTool->addCurve( ls.release() );
+    delete c;
+  }
+  else
+  {
+    mParentTool->addCurve( c );
+  }
 }
 
 void QgsMapToolShapeCircularStringAbstract::clean()

diff --git a/src/gui/qgsmaptoolcapture.cpp b/src/gui/qgsmaptoolcapture.cpp
@@ -806,20 +806,29 @@ int QgsMapToolCapture::addCurve( QgsCurve *c )
     mTempRubberBand->addPoint( endPt ); //add last point of c
   }
 
-  //transform back to layer CRS in case map CRS and layer CRS are different
-  const QgsCoordinateTransform ct = mCanvas->mapSettings().layerTransform( layer() );
-  if ( ct.isValid() )
-  {
-    c->transform( ct, Qgis::TransformDirection::Reverse );
-  }
   const int countBefore = mCaptureCurve.vertexCount();
   //if there is only one point, this the first digitized point that are in the this first curve added --> remove the point
   if ( mCaptureCurve.numPoints() == 1 )
     mCaptureCurve.removeCurve( 0 );
 
-  // we set the extendPrevious option to true to avoid creating compound curves with many 2 vertex linestrings -- instead we prefer
-  // to extend linestring curves so that they continue the previous linestring wherever possible...
-  mCaptureCurve.addCurve( c, !mStartNewCurve );
+  // Transform back to layer CRS in case map CRS and layer CRS are different
+  const QgsCoordinateTransform ct = mCanvas->mapSettings().layerTransform( layer() );
+  if ( ct.isValid() && !ct.isShortCircuited() )
+  {
+    QgsLineString *segmented = c->curveToLine();
+    segmented->transform( ct, Qgis::TransformDirection::Reverse );
+    // Curve geometries will be converted to segments, so we explicitly set extentPrevious to false
+    // to be able to remove the whole curve in undo
+    mCaptureCurve.addCurve( segmented, false );
+    delete c;
+  }
+  else
+  {
+    // we set the extendPrevious option to true to avoid creating compound curves with many 2 vertex linestrings -- instead we prefer
+    // to extend linestring curves so that they continue the previous linestring wherever possible...
+    mCaptureCurve.addCurve( c, !mStartNewCurve );
+  }
+
   mStartNewCurve = false;
 
   const int countAfter = mCaptureCurve.vertexCount();
@@ -872,6 +881,13 @@ void QgsMapToolCapture::undo( bool isAutoRepeat )
     vertexToRemove.part = 0;
     vertexToRemove.ring = 0;
     vertexToRemove.vertex = size() - 1;
+
+    // If the geometry was reprojected, remove the entire last curve.
+    const QgsCoordinateTransform ct = mCanvas->mapSettings().layerTransform( layer() );
+    if ( ct.isValid() && !ct.isShortCircuited() )
+    {
+      mCaptureCurve.removeCurve( mCaptureCurve.nCurves() - 1 );
+    }
     if ( mCaptureCurve.numPoints() == 2 && mCaptureCurve.nCurves() == 1 )
     {
       // store the first vertex to restore if after deleting the curve