Fix selecting features by radius (fix #14748)

(cherry-picked from 33a5ee7)

Author: nyalldawson

python/gui/qgsrubberband.sip

@@ -122,6 +122,14 @@ class QgsRubberBand: QgsMapCanvasItem
      */
     void addPoint( const QgsPoint & p, bool doUpdate = true, int geometryIndex = 0 );
 
+    /** Ensures that a polygon geometry is closed and that the last vertex equals the
+     * first vertex.
+     * @param doUpdate set to true to update the map canvas immediately
+     * @param geometryIndex index of the feature part (in case of multipart geometries)
+     * @note added in QGIS 2.16
+     */
+    void closePoints( bool doUpdate = true, int geometryIndex = 0 );
+
     /**
      * Remove a vertex from the rubberband and (optionally) update canvas.
      * @param index The index of the vertex/point to remove, negative indexes start at end

src/app/qgsmaptoolselectradius.cpp

@@ -109,6 +109,7 @@ void QgsMapToolSelectRadius::setRadiusRubberBand( QgsPoint & radiusEdge )
     double theta = i * ( 2.0 * M_PI / RADIUS_SEGMENTS );
     QgsPoint radiusPoint( mRadiusCenter.x() + r * cos( theta ),
                           mRadiusCenter.y() + r * sin( theta ) );
-    mRubberBand->addPoint( radiusPoint );
+    mRubberBand->addPoint( radiusPoint, false );
   }
+  mRubberBand->closePoints( true );
 }

src/gui/qgsrubberband.cpp

@@ -188,6 +188,26 @@ void QgsRubberBand::addPoint( const QgsPoint & p, bool doUpdate /* = true */, in
   }
 }
 
+void QgsRubberBand::closePoints( bool doUpdate, int geometryIndex )
+{
+  if ( geometryIndex < 0 || geometryIndex >= mPoints.size() )
+  {
+    return;
+  }
+
+  if ( mPoints.at( geometryIndex ).at( 0 ) != mPoints.at( geometryIndex ).at( mPoints.at( geometryIndex ).size() - 1 ) )
+  {
+    mPoints[geometryIndex] << mPoints.at( geometryIndex ).at( 0 );
+  }
+
+  if ( doUpdate )
+  {
+    setVisible( true );
+    updateRect();
+    update();
+  }
+}
+
 
 void QgsRubberBand::removePoint( int index, bool doUpdate/* = true*/, int geometryIndex/* = 0*/ )
 {

src/gui/qgsrubberband.h

@@ -149,6 +149,14 @@ class GUI_EXPORT QgsRubberBand: public QgsMapCanvasItem
      */
     void addPoint( const QgsPoint & p, bool doUpdate = true, int geometryIndex = 0 );
 
+    /** Ensures that a polygon geometry is closed and that the last vertex equals the
+     * first vertex.
+     * @param doUpdate set to true to update the map canvas immediately
+     * @param geometryIndex index of the feature part (in case of multipart geometries)
+     * @note added in QGIS 2.16
+     */
+    void closePoints( bool doUpdate = true, int geometryIndex = 0 );
+
     /**
      * Remove a vertex from the rubberband and (optionally) update canvas.
      * @param index The index of the vertex/point to remove, negative indexes start at end

tests/src/gui/testqgsrubberband.cpp

@@ -46,6 +46,7 @@ class TestQgsRubberband : public QObject
     void testAddSingleMultiGeometries(); //test for #7728
     void testBoundingRect(); //test for #12392
     void testVisibility(); //test for 12486
+    void testClose(); //test closing geometry
 
   private:
     QgsMapCanvas* mCanvas;
@@ -193,6 +194,34 @@ void TestQgsRubberband::testVisibility()
 
 }
 
+void TestQgsRubberband::testClose()
+{
+  QgsRubberBand r( mCanvas, QGis::Polygon );
+
+  // try closing empty rubber band, don't want to crash
+  r.closePoints();
+  QCOMPARE( r.partSize( 0 ), 0 );
+
+  r.addPoint( QgsPoint( 1, 2 ) );
+  r.addPoint( QgsPoint( 1, 3 ) );
+  r.addPoint( QgsPoint( 2, 3 ) );
+  QCOMPARE( r.partSize( 0 ), 3 );
+
+  // test with some bad geometry indexes - don't want to crash!
+  r.closePoints( true, -1 );
+  QCOMPARE( r.partSize( 0 ), 3 );
+  r.closePoints( true, 100 );
+  QCOMPARE( r.partSize( 0 ), 3 );
+
+  // valid close
+  r.closePoints();
+  QCOMPARE( r.partSize( 0 ), 4 );
+
+  // close already closed polygon, should be no change
+  r.closePoints();
+  QCOMPARE( r.partSize( 0 ), 4 );
+}
+
 
 QTEST_MAIN( TestQgsRubberband )
 #include "testqgsrubberband.moc"