Fix QgsRubberBand refresh

- Fixes invalid calculation of rubberband boundingRect
- Scales icon and pen width with rubberband when computing rect
- Recompute rubberband extent on zoom/pan

Fix #12392
Includes testcase

(backport of master commits d43d8bf 84d47c9 a844bfa)

Author: Sandro Santilli

src/gui/qgsrubberband.cpp

@@ -535,7 +535,8 @@ void QgsRubberBand::updateRect()
 
   const QgsMapToPixel& m2p = *( mMapCanvas->getCoordinateTransform() );
 
-  qreal w = ( mIconSize - 1 ) / 2 + mPen.width();
+  qreal res = m2p.mapUnitsPerPixel();
+  qreal w = ( ( mIconSize - 1 ) / 2 + mPen.width() ) / res;
 
   QgsRectangle r;
   for ( int i = 0; i < mPoints.size(); ++i )
@@ -547,14 +548,22 @@ void QgsRubberBand::updateRect()
       QgsPoint p( it->x() + mTranslationOffsetX, it->y() + mTranslationOffsetY );
       p = m2p.transform( p );
       QgsRectangle rect( p.x() - w, p.y() - w, p.x() + w, p.y() + w );
-      r.combineExtentWith( &rect );
+
+      if ( r.isEmpty() )
+      {
+        // Get rectangle of the first point
+        r = rect;
+      }
+      else
+      {
+        r.combineExtentWith( &rect );
+      }
     }
   }
 
   // This is an hack to pass QgsMapCanvasItem::setRect what it
   // expects (encoding of position and size of the item)
   QgsPoint topLeft = m2p.toMapPoint( r.xMinimum(), r.yMinimum() );
-  double res = m2p.mapUnitsPerPixel();
   QgsRectangle rect( topLeft.x(), topLeft.y(), topLeft.x() + r.width()*res, topLeft.y() - r.height()*res );
 
   setRect( rect );
@@ -563,7 +572,12 @@ void QgsRubberBand::updateRect()
 
 void QgsRubberBand::updatePosition( )
 {
-  // nothing to do here...
+  // re-compute rectangle
+  // See http://hub.qgis.org/issues/12392
+  // NOTE: could be optimized by saving map-extent
+  //       of rubberband and simply re-projecting
+  //       that to device-rectange on "updatePosition"
+  updateRect();
 }
 
 void QgsRubberBand::setTranslationOffset( double dx, double dy )

tests/src/gui/testqgsrubberband.cpp

@@ -45,6 +45,7 @@ class TestQgsRubberband : public QObject
     void cleanup(); // will be called after every testfunction.
 
     void testAddSingleMultiGeometries(); //test for #7728
+    void testBoundingRect(); //test for #12392
 
   private:
     QgsMapCanvas* mCanvas;
@@ -107,10 +108,53 @@ void TestQgsRubberband::testAddSingleMultiGeometries()
   QVERIFY( mRubberband->numberOfVertices() == 15 );
 }
 
-QTEST_MAIN( TestQgsRubberband )
-#include "testqgsrubberband.moc"
 
+void TestQgsRubberband::testBoundingRect()
+{
+  QSizeF mapSize = mCanvas->mapSettings().outputSize();
+
+  // Set extent to match canvas size.
+  // This is to ensure a 1:1 scale
+  mCanvas->setExtent( QgsRectangle( QRectF(
+      QPointF(0,0), mapSize
+  ) ) );
+  QCOMPARE( mCanvas->mapUnitsPerPixel (), 1.0 );
+
+  // Polygon extent is 10,10 to 30,30
+  QSharedPointer<QgsGeometry> geom( QgsGeometry::fromWkt(
+      "POLYGON((10 10,10 30,30 30,30 10,10 10))"
+  ) );
+  mRubberband = new QgsRubberBand( mCanvas, mPolygonLayer->geometryType() );
+  mRubberband->setIconSize( 5 ); // default, but better be explicit
+  mRubberband->setWidth( 1 );    // default, but better be explicit
+  mRubberband->addGeometry( geom.data(), mPolygonLayer );
+
+  // 20 pixels for the extent + 3 for pen & icon per side + 2 of padding
+  QCOMPARE( mRubberband->boundingRect(), QRectF(QPointF(-1,-1),QSizeF(28,28)) );
+  QCOMPARE( mRubberband->pos(), QPointF(
+    // 10 for extent minx - 3 for pen & icon
+    7,
+    // 30 for extent maxy - 3 for pen & icon
+    mapSize.height() - 30 - 3
+  ) );
+
+  mCanvas->setExtent( QgsRectangle( QRectF(
+      QPointF(0,0), mapSize/2
+  ) ) );
+
+  // 40 pixels for the extent + 6 for pen & icon per side + 2 of padding
+  QCOMPARE( mRubberband->boundingRect(), QRectF(QPointF(-1,-1),QSizeF(54,54)) );
+  QCOMPARE( mRubberband->pos(), QPointF(
+    // 10 for extent minx - 3 for pen & icon
+    7 * 2,
+    // 30 for extent maxy - 3 for pen & icon
+    mapSize.height() - ( 30 + 3 ) * 2
+  ) );
+
+}
 
 
+QTEST_MAIN( TestQgsRubberband )
+#include "testqgsrubberband.moc"