Remove -1 return code for QgsMapToolCapture::addVertex

This code corresponded to a "not a vector layer" error, which needlessly prevents the QgsMapToolCapture class from being usable as a base class for map tools which operate on non-vector layers.
qgis · Sep 8, 2021 · c8ac971 · c8ac971
1 parent b6c032d
commit c8ac971
Show file tree

Hide file tree

Showing 5 changed files with 107 additions and 7 deletions.
diff --git a/python/gui/auto_generated/qgsmaptoolcapture.sip.in b/python/gui/auto_generated/qgsmaptoolcapture.sip.in
@@ -219,7 +219,7 @@ WkbType of the current layer).
 %Docstring
 Adds a point to the rubber band (in map coordinates) and to the capture list (in layer coordinates)
 
-:return: 0 in case of success, 1 if current layer is not a vector layer, 2 if coordinate transformation failed
+:return: 0 in case of success, 2 if coordinate transformation failed
 %End
 
     int addVertex( const QgsPointXY &mapPoint, const QgsPointLocator::Match &match );

diff --git a/src/gui/qgsmaptoolcapture.cpp b/src/gui/qgsmaptoolcapture.cpp
@@ -557,17 +557,24 @@ int QgsMapToolCapture::addVertex( const QgsPointXY &point, const QgsPointLocator
   if ( mCapturing && mStreamingEnabled && !mAllowAddingStreamingPoints )
     return 0;
 
-  int res;
+  int res = 0;
   QgsPoint layerPoint;
-  res = fetchLayerPoint( match, layerPoint );
-  if ( res != 0 )
+  if ( mCanvas->currentLayer() && mCanvas->currentLayer()->type() == QgsMapLayerType::VectorLayer )
   {
-    res = nextPoint( QgsPoint( point ), layerPoint );
+    res = fetchLayerPoint( match, layerPoint );
     if ( res != 0 )
     {
-      return res;
+      res = nextPoint( QgsPoint( point ), layerPoint );
+      if ( res != 0 )
+      {
+        return res;
+      }
     }
   }
+  else
+  {
+    layerPoint = QgsPoint( point );
+  }
   const QgsPoint mapPoint = toMapCoordinates( canvas()->currentLayer(), layerPoint );
 
   if ( mCaptureMode == CapturePoint )

diff --git a/src/gui/qgsmaptoolcapture.h b/src/gui/qgsmaptoolcapture.h
@@ -314,7 +314,7 @@ class GUI_EXPORT QgsMapToolCapture : public QgsMapToolAdvancedDigitizing
 
     /**
      * Adds a point to the rubber band (in map coordinates) and to the capture list (in layer coordinates)
-     * \returns 0 in case of success, 1 if current layer is not a vector layer, 2 if coordinate transformation failed
+     * \returns 0 in case of success, 2 if coordinate transformation failed
      */
     int addVertex( const QgsPointXY &point );
 
@@ -475,6 +475,8 @@ class GUI_EXPORT QgsMapToolCapture : public QgsMapToolAdvancedDigitizing
 
     bool mIgnoreSubsequentAutoRepeatUndo = false;
 
+    friend class TestQgsMapToolCapture;
+
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS( QgsMapToolCapture::Capabilities )

diff --git a/tests/src/gui/CMakeLists.txt b/tests/src/gui/CMakeLists.txt
@@ -40,6 +40,7 @@ set(TESTS
   testqgsfocuswatcher.cpp
   testqgshtmlwidgetwrapper.cpp
   testqgsmapcanvas.cpp
+  testqgsmaptoolcapture.cpp
   testqgsmessagebar.cpp
   testprojectionissues.cpp
   testqgsgui.cpp

diff --git a/tests/src/gui/testqgsmaptoolcapture.cpp b/tests/src/gui/testqgsmaptoolcapture.cpp
@@ -0,0 +1,90 @@
+/***************************************************************************
+    testqgsmaptooledit.cpp
+     --------------------------------------
+    Date                 : September 2021
+    Copyright            : (C) 2021 Nyall Dawson
+    Email                : nyall dot dawson at gmail dot com
+ ***************************************************************************
+ *                                                                         *
+ *   This program is free software; you can redistribute it and/or modify  *
+ *   it under the terms of the GNU General Public License as published by  *
+ *   the Free Software Foundation; either version 2 of the License, or     *
+ *   (at your option) any later version.                                   *
+ *                                                                         *
+ ***************************************************************************/
+#include <QCoreApplication>
+
+#include "qgstest.h"
+#include "qgsguiutils.h"
+#include "qgsmaptoolcapture.h"
+#include "qgsapplication.h"
+#include "qgsmapcanvas.h"
+#include "qgslogger.h"
+#include "qgsannotationlayer.h"
+#include "qgsadvanceddigitizingdockwidget.h"
+
+class TestQgsMapToolCapture : public QObject
+{
+    Q_OBJECT
+  public:
+    TestQgsMapToolCapture() = default;
+
+  private slots:
+    void initTestCase(); // will be called before the first testfunction is executed.
+    void cleanupTestCase(); // will be called after the last testfunction was executed.
+    void init(); // will be called before each testfunction is executed.
+    void cleanup(); // will be called after every testfunction.
+
+    void addVertexNonVectorLayer();
+
+};
+
+void TestQgsMapToolCapture::initTestCase()
+{
+  QgsApplication::init();
+  QgsApplication::initQgis();
+  QgsApplication::showSettings();
+}
+
+void TestQgsMapToolCapture::cleanupTestCase()
+{
+  QgsApplication::exitQgis();
+}
+
+void TestQgsMapToolCapture::init()
+{
+}
+
+void TestQgsMapToolCapture::cleanup()
+{
+}
+
+void TestQgsMapToolCapture::addVertexNonVectorLayer()
+{
+  QgsProject::instance()->clear();
+  QgsMapCanvas canvas;
+  canvas.setDestinationCrs( QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:4326" ) ) );
+  canvas.setFrameStyle( QFrame::NoFrame );
+  canvas.resize( 600, 600 );
+  canvas.setExtent( QgsRectangle( 0, 0, 10, 10 ) );
+  canvas.show(); // to make the canvas resize
+
+  QgsAnnotationLayer *layer = new QgsAnnotationLayer( QStringLiteral( "test" ), QgsAnnotationLayer::LayerOptions( QgsProject::instance()->transformContext() ) );
+  QVERIFY( layer->isValid() );
+  QgsProject::instance()->addMapLayers( { layer} );
+
+  canvas.setLayers( { layer } );
+  canvas.setCurrentLayer( layer );
+
+  QgsAdvancedDigitizingDockWidget cadDock( &canvas );
+  QgsMapToolCapture tool( &canvas, &cadDock, QgsMapToolCapture::CaptureLine );
+  canvas.setMapTool( &tool );
+
+  // even though we don't have a vector layer selected, adding vertices should still be allowed
+  QCOMPARE( tool.addVertex( QgsPoint( 5, 5 ), QgsPointLocator::Match() ), 0 );
+
+
+}
+
+QGSTEST_MAIN( TestQgsMapToolCapture )
+#include "testqgsmaptoolcapture.moc"