Move logic for maintaining exact extent when CRS changes from

raster layer save as dialog to QgsExtentGroupBox, add tests
qgis · May 31, 2017 · 9e14741 · 9e14741
1 parent af029c5
commit 9e14741
Show file tree

Hide file tree

Showing 5 changed files with 112 additions and 51 deletions.
diff --git a/python/gui/qgsextentgroupbox.sip b/python/gui/qgsextentgroupbox.sip
@@ -67,32 +67,34 @@ class QgsExtentGroupBox : QgsCollapsibleGroupBox
     void setCurrentExtent( const QgsRectangle &currentExtent, const QgsCoordinateReferenceSystem &currentCrs );
 %Docstring
  Sets the current extent to show in the widget - should be called as part of initialization (or whenever current extent changes).
+ The current extent is usually set to match the current map canvas extent.
 .. seealso:: currentExtent()
 .. seealso:: currentCrs()
 %End
 
     QgsRectangle currentExtent() const;
 %Docstring
- Returns the current extent set for the widget.
+ Returns the current extent set for the widget. The current extent is usually set to match the
+ current map canvas extent.
 .. seealso:: setCurrentExtent()
 .. seealso:: currentCrs()
  :rtype: QgsRectangle
 %End
 
     QgsCoordinateReferenceSystem currentCrs() const;
 %Docstring
- Returns the coordinate reference system for the current extent set for the widget.
+ Returns the coordinate reference system for the current extent set for the widget. The current
+ extent and CRS usually reflects the map canvas extent and CRS.
 .. seealso:: setCurrentExtent()
 .. seealso:: currentExtent()
  :rtype: QgsCoordinateReferenceSystem
 %End
 
-    void setOutputCrs( const QgsCoordinateReferenceSystem &outputCrs, bool reprojectCurrentExtent = true );
+    void setOutputCrs( const QgsCoordinateReferenceSystem &outputCrs );
 %Docstring
  Sets the output CRS - may need to be used for transformation from original/current extent.
  Should be called as part of initialization and whenever the the output CRS is changed.
- If ``reprojectCurrentExtent`` is true then the current extent will be reproject into the
- new output CRS.
+ The current extent will be reprojected into the new output CRS.
 %End
 
     QgsRectangle outputExtent() const;

diff --git a/src/gui/qgsextentgroupbox.cpp b/src/gui/qgsextentgroupbox.cpp
@@ -62,25 +62,45 @@ void QgsExtentGroupBox::setCurrentExtent( const QgsRectangle &currentExtent, con
   mCurrentCrs = currentCrs;
 }
 
-void QgsExtentGroupBox::setOutputCrs( const QgsCoordinateReferenceSystem &outputCrs, bool reprojectCurrentExtent )
+void QgsExtentGroupBox::setOutputCrs( const QgsCoordinateReferenceSystem &outputCrs )
 {
-  if ( reprojectCurrentExtent && mOutputCrs != outputCrs )
+  if ( mOutputCrs != outputCrs )
   {
-    try
+    switch ( mExtentState )
     {
-      QgsCoordinateTransform ct( mOutputCrs, outputCrs );
-      QgsRectangle extent = ct.transformBoundingBox( outputExtent() );
-      mXMinLineEdit->setText( QgsRasterBlock::printValue( extent.xMinimum() ) );
-      mXMaxLineEdit->setText( QgsRasterBlock::printValue( extent.xMaximum() ) );
-      mYMinLineEdit->setText( QgsRasterBlock::printValue( extent.yMinimum() ) );
-      mYMaxLineEdit->setText( QgsRasterBlock::printValue( extent.yMaximum() ) );
-    }
-    catch ( QgsCsException & )
-    {
-      // can't reproject
+      case CurrentExtent:
+        mOutputCrs = outputCrs;
+        setOutputExtentFromCurrent();
+        break;
+
+      case OriginalExtent:
+        mOutputCrs = outputCrs;
+        setOutputExtentFromOriginal();
+        break;
+
+      case ProjectLayerExtent:
+        mOutputCrs = outputCrs;
+        setOutputExtentFromLayer( mExtentLayer.data() );
+        break;
+
+      case UserExtent:
+        try
+        {
+          QgsCoordinateTransform ct( mOutputCrs, outputCrs );
+          QgsRectangle extent = ct.transformBoundingBox( outputExtent() );
+          mOutputCrs = outputCrs;
+          setOutputExtentFromUser( extent, outputCrs );
+        }
+        catch ( QgsCsException & )
+        {
+          // can't reproject
+          mOutputCrs = outputCrs;
+        }
+        break;
     }
+
   }
-  mOutputCrs = outputCrs;
+
 }
 
 void QgsExtentGroupBox::setOutputExtent( const QgsRectangle &r, const QgsCoordinateReferenceSystem &srcCrs, ExtentState state )
@@ -168,7 +188,7 @@ void QgsExtentGroupBox::layerMenuAboutToShow()
     QAction *act = new QAction( icon, text, mLayerMenu );
     act->setToolTip( mMapLayerModel->data( index, Qt::ToolTipRole ).toString() );
     QString layerId = mMapLayerModel->data( index, QgsMapLayerModel::LayerIdRole ).toString();
-    if ( mExtentState == ProjectLayerExtent && mExtentLayerId == layerId )
+    if ( mExtentState == ProjectLayerExtent && mExtentLayer && mExtentLayer->id() == layerId )
     {
       act->setCheckable( true );
       act->setChecked( true );
@@ -212,7 +232,7 @@ void QgsExtentGroupBox::setOutputExtentFromLayer( const QgsMapLayer *layer )
   if ( !layer )
     return;
 
-  mExtentLayerId = layer->id();
+  mExtentLayer = layer;
   mExtentLayerName = layer->name();
 
   setOutputExtent( layer->extent(), layer->crs(), ProjectLayerExtent );

diff --git a/src/gui/qgsextentgroupbox.h b/src/gui/qgsextentgroupbox.h
@@ -82,20 +82,23 @@ class GUI_EXPORT QgsExtentGroupBox : public QgsCollapsibleGroupBox, private Ui::
 
     /**
      * Sets the current extent to show in the widget - should be called as part of initialization (or whenever current extent changes).
+     * The current extent is usually set to match the current map canvas extent.
      * \see currentExtent()
      * \see currentCrs()
      */
     void setCurrentExtent( const QgsRectangle &currentExtent, const QgsCoordinateReferenceSystem &currentCrs );
 
     /**
-     * Returns the current extent set for the widget.
+     * Returns the current extent set for the widget. The current extent is usually set to match the
+     * current map canvas extent.
      * \see setCurrentExtent()
      * \see currentCrs()
      */
     QgsRectangle currentExtent() const { return mCurrentExtent; }
 
     /**
-     * Returns the coordinate reference system for the current extent set for the widget.
+     * Returns the coordinate reference system for the current extent set for the widget. The current
+     * extent and CRS usually reflects the map canvas extent and CRS.
      * \see setCurrentExtent()
      * \see currentExtent()
      */
@@ -104,10 +107,9 @@ class GUI_EXPORT QgsExtentGroupBox : public QgsCollapsibleGroupBox, private Ui::
     /**
      * Sets the output CRS - may need to be used for transformation from original/current extent.
      * Should be called as part of initialization and whenever the the output CRS is changed.
-     * If \a reprojectCurrentExtent is true then the current extent will be reproject into the
-     * new output CRS.
+     * The current extent will be reprojected into the new output CRS.
      */
-    void setOutputCrs( const QgsCoordinateReferenceSystem &outputCrs, bool reprojectCurrentExtent = true );
+    void setOutputCrs( const QgsCoordinateReferenceSystem &outputCrs );
 
     /**
      * Returns the extent shown in the widget - in output CRS coordinates.
@@ -194,7 +196,7 @@ class GUI_EXPORT QgsExtentGroupBox : public QgsCollapsibleGroupBox, private Ui::
     QMenu *mLayerMenu = nullptr;
     QgsMapLayerModel *mMapLayerModel = nullptr;
     QList< QAction * > mMenuActions;
-    QString mExtentLayerId;
+    QPointer< const QgsMapLayer > mExtentLayer;
     QString mExtentLayerName;
 
     void setExtentToLayerExtent( const QString &layerId );

diff --git a/src/gui/qgsrasterlayersaveasdialog.cpp b/src/gui/qgsrasterlayersaveasdialog.cpp
@@ -435,24 +435,7 @@ void QgsRasterLayerSaveAsDialog::crsChanged()
 {
   if ( outputCrs() != mPreviousCrs )
   {
-    mExtentGroupBox->setOutputCrs( outputCrs(), false );
-    QgsExtentGroupBox::ExtentState state = mExtentGroupBox->extentState();
-
-    // Reset extent
-    // We could reproject previous but that would add additional space also if
-    // it is was not necessary or at leas it could decrease accuracy
-    if ( state == QgsExtentGroupBox::OriginalExtent )
-    {
-      mExtentGroupBox->setOutputExtentFromOriginal();
-    }
-    else if ( state == QgsExtentGroupBox::CurrentExtent )
-    {
-      mExtentGroupBox->setOutputExtentFromCurrent();
-    }
-    else
-    {
-      mExtentGroupBox->setOutputExtentFromUser( mExtentGroupBox->outputExtent(), mPreviousCrs );
-    }
+    mExtentGroupBox->setOutputCrs( outputCrs() );
 
     // Reset resolution
     if ( mResolutionRadioButton->isChecked() )

diff --git a/tests/src/python/test_qgsextentgroupbox.py b/tests/src/python/test_qgsextentgroupbox.py
@@ -15,7 +15,7 @@
 import qgis  # NOQA
 import os
 
-from qgis.core import QgsRectangle, QgsCoordinateReferenceSystem, QgsVectorLayer, QgsProject
+from qgis.core import QgsRectangle, QgsCoordinateReferenceSystem, QgsVectorLayer, QgsProject, QgsFeature, QgsGeometry
 from qgis.gui import QgsExtentGroupBox
 
 from qgis.PyQt.QtTest import QSignalSpy
@@ -82,6 +82,8 @@ def test_SettingExtent(self):
         self.assertEqual(w.extentState(), QgsExtentGroupBox.ProjectLayerExtent)
         self.assertEqual(len(spy), 4)
 
+        QgsProject.instance().removeAllMapLayers()
+
     def testSetOutputCrs(self):
         w = qgis.gui.QgsExtentGroupBox()
 
@@ -90,14 +92,66 @@ def testSetOutputCrs(self):
         w.setOutputExtentFromCurrent()
         self.assertEqual(w.outputExtent(), QgsRectangle(1, 2, 3, 4))
 
-        # no reprojection
-        w.setOutputCrs(QgsCoordinateReferenceSystem('epsg:3785'), False)
+        # with reprojection
+        w.setOutputCrs(QgsCoordinateReferenceSystem('epsg:3785'))
+        self.assertEqual(w.outputExtent().toString(4), QgsRectangle(111319.4908, 222684.2085, 333958.4724, 445640.1097).toString(4))
+        # change CRS back
+        w.setOutputCrs(QgsCoordinateReferenceSystem('epsg:4326'))
+        # extent should be back to current - not a reprojection of the reprojected bounds
+        self.assertEqual(w.outputExtent().toString(20), QgsRectangle(1, 2, 3, 4).toString(20))
+
+        # repeat, this time using original extents
+        w = qgis.gui.QgsExtentGroupBox()
+
+        w.setOutputCrs(QgsCoordinateReferenceSystem('epsg:4326'))
+        w.setOriginalExtent(QgsRectangle(1, 2, 3, 4), QgsCoordinateReferenceSystem('epsg:4326'))
+        w.setOutputExtentFromOriginal()
         self.assertEqual(w.outputExtent(), QgsRectangle(1, 2, 3, 4))
-        w.setOutputCrs(QgsCoordinateReferenceSystem('epsg:4326'), False)
 
         # with reprojection
-        w.setOutputCrs(QgsCoordinateReferenceSystem('epsg:3785'), True)
-        self.assertEqual(w.outputExtent().toString(4), QgsRectangle(111319.4908, 222684.2085, 333958.4724, 445640.1097).toString(4))
+        w.setOutputCrs(QgsCoordinateReferenceSystem('epsg:3785'))
+        self.assertEqual(w.outputExtent().toString(4),
+                         QgsRectangle(111319.4908, 222684.2085, 333958.4724, 445640.1097).toString(4))
+        # change CRS back
+        w.setOutputCrs(QgsCoordinateReferenceSystem('epsg:4326'))
+        # extent should be back to original - not a reprojection of the reprojected bounds
+        self.assertEqual(w.outputExtent().toString(20), QgsRectangle(1, 2, 3, 4).toString(20))
+
+        # repeat, this time using layer extent
+        layer = QgsVectorLayer("Polygon?crs=4326", 'memory', 'memory')
+        self.assertTrue(layer.isValid())
+        f = QgsFeature()
+        f.setGeometry(QgsGeometry.fromWkt('Polygon((1 2, 3 2, 3 4, 1 4, 1 2))'))
+        layer.dataProvider().addFeatures([f])
+        QgsProject.instance().addMapLayer(layer)
+        w.setOutputCrs(QgsCoordinateReferenceSystem('epsg:4326'))
+        w.setOutputExtentFromLayer(layer)
+        self.assertEqual(w.outputExtent(), QgsRectangle(1, 2, 3, 4))
+
+        w.setOutputCrs(QgsCoordinateReferenceSystem('epsg:3785'))
+        self.assertEqual(w.outputExtent().toString(4),
+                         QgsRectangle(111319.4908, 222684.2085, 333958.4724, 445640.1097).toString(4))
+        # change CRS back
+        w.setOutputCrs(QgsCoordinateReferenceSystem('epsg:4326'))
+        # extent should be back to original - not a reprojection of the reprojected bounds
+        self.assertEqual(w.outputExtent().toString(20), QgsRectangle(1, 2, 3, 4).toString(20))
+
+        # custom extent
+        w = qgis.gui.QgsExtentGroupBox()
+
+        w.setOutputCrs(QgsCoordinateReferenceSystem('epsg:4326'))
+        w.setOutputExtentFromUser(QgsRectangle(1, 2, 3, 4), QgsCoordinateReferenceSystem('epsg:4326'))
+        self.assertEqual(w.outputExtent(), QgsRectangle(1, 2, 3, 4))
+
+        # with reprojection
+        w.setOutputCrs(QgsCoordinateReferenceSystem('epsg:3785'))
+        self.assertEqual(w.outputExtent().toString(4),
+                         QgsRectangle(111319.4908, 222684.2085, 333958.4724, 445640.1097).toString(4))
+        # change CRS back
+        w.setOutputCrs(QgsCoordinateReferenceSystem('epsg:4326'))
+        # in this case we can't retrieve the original user extent in 4326, so we have a reprojection of the reprojected bounds
+        # just test this by restricting the test to 4 decimals
+        self.assertEqual(w.outputExtent().toString(4), QgsRectangle(1, 2, 3, 4).toString(4))
 
 
 if __name__ == '__main__':