From 75efa28df04f1fd1b61c31420e7c1ba19dc97730 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 22 Sep 2020 15:38:26 +0200 Subject: [PATCH 1/3] Respect scale lock when panning/zooming the map Fixes #38953 and #38417 --- python/gui/auto_generated/qgsmapcanvas.sip.in | 1 + src/core/qgsmapsettings.cpp | 1 + src/gui/qgsmapcanvas.cpp | 34 ++++++--- src/gui/qgsmapcanvas.h | 22 ++++++ tests/src/python/test_qgsmapcanvas.py | 74 +++++++++++++++++++ 5 files changed, 122 insertions(+), 10 deletions(-) diff --git a/python/gui/auto_generated/qgsmapcanvas.sip.in b/python/gui/auto_generated/qgsmapcanvas.sip.in index d7e32cfec642..60b7bc6fc2ef 100644 --- a/python/gui/auto_generated/qgsmapcanvas.sip.in +++ b/python/gui/auto_generated/qgsmapcanvas.sip.in @@ -1201,6 +1201,7 @@ called when panning is in action, reset indicates end of panning called on resize or changed extent to notify canvas items to change their rectangle %End + public: }; // class QgsMapCanvas diff --git a/src/core/qgsmapsettings.cpp b/src/core/qgsmapsettings.cpp index 738367b3025b..0c2ba10cd5cd 100644 --- a/src/core/qgsmapsettings.cpp +++ b/src/core/qgsmapsettings.cpp @@ -226,6 +226,7 @@ void QgsMapSettings::updateDerived() QgsDebugMsgLevel( QStringLiteral( "Rotation: %1 degrees" ).arg( mRotation ), 5 ); QgsDebugMsgLevel( QStringLiteral( "Extent: %1" ).arg( mExtent.asWktCoordinates() ), 5 ); QgsDebugMsgLevel( QStringLiteral( "Visible Extent: %1" ).arg( mVisibleExtent.asWktCoordinates() ), 5 ); + QgsDebugMsgLevel( QStringLiteral( "Magnification factor: %1" ).arg( mMagnificationFactor ), 5 ); mValid = true; } diff --git a/src/gui/qgsmapcanvas.cpp b/src/gui/qgsmapcanvas.cpp index 6156c812e914..56928aa49b28 100644 --- a/src/gui/qgsmapcanvas.cpp +++ b/src/gui/qgsmapcanvas.cpp @@ -1086,7 +1086,23 @@ void QgsMapCanvas::setExtent( const QgsRectangle &r, bool magnified ) } else { - mSettings.setExtent( r, magnified ); + // If scale is locked we need to maintain the current scale, so we + // - magnify and recenter the map + // - restore locked scale + if ( mScaleLocked && magnified ) + { + ScaleRestorer restorer( this ); + const double ratio { extent().width() / extent().height() }; + const double factor { r.width() / r.height() > ratio ? extent().width() / r.width() : extent().height() / r.height() }; + const double scaleFactor { qBound( QgsGuiUtils::CANVAS_MAGNIFICATION_MIN, mSettings.magnificationFactor() * factor, QgsGuiUtils::CANVAS_MAGNIFICATION_MAX ) }; + const QgsPointXY newCenter { r.center() }; + mSettings.setMagnificationFactor( scaleFactor, &newCenter ); + emit magnificationChanged( scaleFactor ); + } + else + { + mSettings.setExtent( r, magnified ); + } } emit extentsChanged(); updateScale(); @@ -1134,18 +1150,15 @@ bool QgsMapCanvas::setReferencedExtent( const QgsReferencedRectangle &extent ) void QgsMapCanvas::setCenter( const QgsPointXY ¢er ) { const QgsRectangle r = mapSettings().extent(); - const double x = center.x(); - const double y = center.y(); + const double xMin = center.x() - r.width() / 2.0; + const double yMin = center.y() - r.height() / 2.0; const QgsRectangle rect( - x - r.width() / 2.0, y - r.height() / 2.0, - x + r.width() / 2.0, y + r.height() / 2.0 + xMin, yMin, + xMin + r.width(), yMin + r.height() ); if ( ! rect.isEmpty() ) { - setExtent( - rect, - true - ); + setExtent( rect, true ); } } // setCenter @@ -1929,6 +1942,7 @@ void QgsMapCanvas::zoomWithCenter( int x, int y, bool zoomIn ) if ( mScaleLocked ) { + ScaleRestorer restorer( this ); setMagnificationFactor( mapSettings().magnificationFactor() / scaleFactor, ¢er ); } else @@ -2511,7 +2525,7 @@ void QgsMapCanvas::zoomByFactor( double scaleFactor, const QgsPointXY *center, b { if ( mScaleLocked && !ignoreScaleLock ) { - // zoom map to mouse cursor by magnifying + ScaleRestorer restorer( this ); setMagnificationFactor( mapSettings().magnificationFactor() / scaleFactor, center ); } else diff --git a/src/gui/qgsmapcanvas.h b/src/gui/qgsmapcanvas.h index 7ea7357573de..0ace3ca919f2 100644 --- a/src/gui/qgsmapcanvas.h +++ b/src/gui/qgsmapcanvas.h @@ -1110,6 +1110,28 @@ class GUI_EXPORT QgsMapCanvas : public QGraphicsView private: + // Restore scale RAII + class ScaleRestorer + { + public: + ScaleRestorer( QgsMapCanvas *canvas ): + mCanvas( canvas ) + { + mLockedScale = mCanvas->mapSettings().scale(); + }; + + ~ScaleRestorer() + { + QgsRectangle newExtent = mCanvas->mapSettings().extent(); + newExtent.scale( mLockedScale / mCanvas->mapSettings().scale() ); + mCanvas->mSettings.setExtent( newExtent ); + }; + + private: + QgsMapCanvas *mCanvas; + double mLockedScale; + }; + //! encompases all map settings necessary for map rendering QgsMapSettings mSettings; diff --git a/tests/src/python/test_qgsmapcanvas.py b/tests/src/python/test_qgsmapcanvas.py index 00fb416b67b9..1c99b65af031 100644 --- a/tests/src/python/test_qgsmapcanvas.py +++ b/tests/src/python/test_qgsmapcanvas.py @@ -26,6 +26,7 @@ QgsPolygon, QgsLineString, QgsPoint, + QgsPointXY, QgsApplication) from qgis.gui import (QgsMapCanvas) @@ -485,6 +486,79 @@ def testSaveMultipleCanvasesToProject(self): self.assertEqual(c4.expressionContextScope().variable('vara'), 2222) self.assertEqual(c4.expressionContextScope().variable('varc'), 'cc') + def testLockedScale(self): + """Test zoom/pan/center operations when scale lock is on""" + + c = QgsMapCanvas() + c.setExtent(QgsRectangle(5, 45, 9, 47)) + c.zoomScale(2500000) + c.setScaleLocked(True) + + # Test setExtent + c.setExtent(QgsRectangle(6, 45.5, 8, 46), True) + self.assertEqual(round(c.scale()), 2500000) + self.assertEqual(c.center().x(), 7.0) + self.assertEqual(c.center().y(), 45.75) + self.assertEqual(round(c.magnificationFactor(), 1), 1.9) + + # Test setCenter + c.setCenter(QgsPointXY(6, 46)) + self.assertEqual(c.center().x(), 6) + self.assertEqual(c.center().y(), 46) + self.assertEqual(round(c.scale()), 2500000) + + # Test zoom + c.zoomByFactor(0.5, QgsPointXY(6.5, 46.5), False) + self.assertEqual(c.center().x(), 6.5) + self.assertEqual(c.center().y(), 46.5) + self.assertEqual(round(c.magnificationFactor(), 1), 3.9) + self.assertEqual(round(c.scale()), 2500000) + + # Test zoom with center + # default zoom factor is 2, x and y are pixel coordinates, default size is 640x480 + c.zoomWithCenter(300, 200, True) + self.assertEqual(round(c.center().x(), 1), 6.5) + self.assertEqual(round(c.center().y(), 1), 46.6) + self.assertEqual(round(c.scale()), 2500000) + self.assertEqual(round(c.magnificationFactor(), 1), 7.8) + # out ... + c.zoomWithCenter(300, 200, False) + self.assertEqual(round(c.center().x(), 1), 6.5) + self.assertEqual(round(c.center().y(), 1), 46.6) + self.assertEqual(round(c.scale()), 2500000) + self.assertEqual(round(c.magnificationFactor(), 1), 3.9) + + # Test setExtent with different ratio + c2 = QgsMapCanvas() + c2.setExtent(QgsRectangle(5, 45, 9, 47)) + c2.zoomScale(2500000) + c2.setScaleLocked(True) + + c2.setExtent(QgsRectangle(3, 45, 11, 45.5), True) + self.assertEqual(round(c2.scale()), 2500000) + self.assertEqual(c2.center().x(), 7.0) + self.assertEqual(c2.center().y(), 45.25) + self.assertEqual(round(c2.magnificationFactor(), 1), 0.5) + + # Restore original + c2.setExtent(QgsRectangle(5, 45, 9, 47), True) + self.assertEqual(round(c2.scale()), 2500000) + self.assertEqual(c2.center().x(), 7.0) + self.assertEqual(c2.center().y(), 46.0) + self.assertEqual(round(c2.magnificationFactor(), 1), 1) + + c2.setExtent(QgsRectangle(7, 46, 11, 46.5), True) + self.assertEqual(round(c2.scale()), 2500000) + self.assertEqual(c2.center().x(), 9.0) + self.assertEqual(c2.center().y(), 46.25) + self.assertEqual(round(c2.magnificationFactor(), 1), 1) + + c2.setExtent(QgsRectangle(7, 46, 9, 46.5), True) + self.assertEqual(round(c2.scale()), 2500000) + self.assertEqual(c2.center().x(), 8.0) + self.assertEqual(c2.center().y(), 46.25) + self.assertEqual(round(c2.magnificationFactor(), 1), 2) + if __name__ == '__main__': unittest.main() From dc1951084d760a949a6c471f94bcfa9eeb4b093a Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Wed, 23 Sep 2020 16:18:31 +0200 Subject: [PATCH 2/3] Test debug code --- tests/src/python/test_qgsmapcanvas.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/src/python/test_qgsmapcanvas.py b/tests/src/python/test_qgsmapcanvas.py index 1c99b65af031..9445c9b4883f 100644 --- a/tests/src/python/test_qgsmapcanvas.py +++ b/tests/src/python/test_qgsmapcanvas.py @@ -490,6 +490,9 @@ def testLockedScale(self): """Test zoom/pan/center operations when scale lock is on""" c = QgsMapCanvas() + self.assertEqual(c.size().width(), 640) + self.assertEqual(c.size().height(), 480) + c.setExtent(QgsRectangle(5, 45, 9, 47)) c.zoomScale(2500000) c.setScaleLocked(True) From 117a477fd2358b8076c6ebeaf8174d64251604d9 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Wed, 23 Sep 2020 18:45:39 +0200 Subject: [PATCH 3/3] Adjust test for DPIs --- tests/src/python/test_qgsmapcanvas.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/src/python/test_qgsmapcanvas.py b/tests/src/python/test_qgsmapcanvas.py index 9445c9b4883f..02f45d82091d 100644 --- a/tests/src/python/test_qgsmapcanvas.py +++ b/tests/src/python/test_qgsmapcanvas.py @@ -490,19 +490,22 @@ def testLockedScale(self): """Test zoom/pan/center operations when scale lock is on""" c = QgsMapCanvas() + dpr = c.mapSettings().devicePixelRatio() self.assertEqual(c.size().width(), 640) self.assertEqual(c.size().height(), 480) c.setExtent(QgsRectangle(5, 45, 9, 47)) + self.assertEqual(round(c.scale() / 100000), 13 * dpr) c.zoomScale(2500000) c.setScaleLocked(True) + self.assertEqual(round(c.magnificationFactor(), 1), 1) # Test setExtent c.setExtent(QgsRectangle(6, 45.5, 8, 46), True) self.assertEqual(round(c.scale()), 2500000) self.assertEqual(c.center().x(), 7.0) self.assertEqual(c.center().y(), 45.75) - self.assertEqual(round(c.magnificationFactor(), 1), 1.9) + self.assertEqual(round(c.magnificationFactor()), 4 / dpr) # Test setCenter c.setCenter(QgsPointXY(6, 46)) @@ -514,7 +517,7 @@ def testLockedScale(self): c.zoomByFactor(0.5, QgsPointXY(6.5, 46.5), False) self.assertEqual(c.center().x(), 6.5) self.assertEqual(c.center().y(), 46.5) - self.assertEqual(round(c.magnificationFactor(), 1), 3.9) + self.assertTrue(c.magnificationFactor() > 7 / dpr) self.assertEqual(round(c.scale()), 2500000) # Test zoom with center @@ -523,13 +526,13 @@ def testLockedScale(self): self.assertEqual(round(c.center().x(), 1), 6.5) self.assertEqual(round(c.center().y(), 1), 46.6) self.assertEqual(round(c.scale()), 2500000) - self.assertEqual(round(c.magnificationFactor(), 1), 7.8) + self.assertTrue(c.magnificationFactor() > (14 / dpr) and c.magnificationFactor() < (16 / dpr)) # out ... c.zoomWithCenter(300, 200, False) self.assertEqual(round(c.center().x(), 1), 6.5) self.assertEqual(round(c.center().y(), 1), 46.6) self.assertEqual(round(c.scale()), 2500000) - self.assertEqual(round(c.magnificationFactor(), 1), 3.9) + self.assertTrue(c.magnificationFactor() > 7 / dpr) # Test setExtent with different ratio c2 = QgsMapCanvas() @@ -541,26 +544,26 @@ def testLockedScale(self): self.assertEqual(round(c2.scale()), 2500000) self.assertEqual(c2.center().x(), 7.0) self.assertEqual(c2.center().y(), 45.25) - self.assertEqual(round(c2.magnificationFactor(), 1), 0.5) + self.assertAlmostEqual(c2.magnificationFactor(), 1 / dpr, 0) # Restore original c2.setExtent(QgsRectangle(5, 45, 9, 47), True) self.assertEqual(round(c2.scale()), 2500000) self.assertEqual(c2.center().x(), 7.0) self.assertEqual(c2.center().y(), 46.0) - self.assertEqual(round(c2.magnificationFactor(), 1), 1) + self.assertAlmostEqual(c2.magnificationFactor(), 2 / dpr, 0) c2.setExtent(QgsRectangle(7, 46, 11, 46.5), True) self.assertEqual(round(c2.scale()), 2500000) self.assertEqual(c2.center().x(), 9.0) self.assertEqual(c2.center().y(), 46.25) - self.assertEqual(round(c2.magnificationFactor(), 1), 1) + self.assertAlmostEqual(c2.magnificationFactor(), 2 / dpr, 0) c2.setExtent(QgsRectangle(7, 46, 9, 46.5), True) self.assertEqual(round(c2.scale()), 2500000) self.assertEqual(c2.center().x(), 8.0) self.assertEqual(c2.center().y(), 46.25) - self.assertEqual(round(c2.magnificationFactor(), 1), 2) + self.assertAlmostEqual(c2.magnificationFactor(), 4 / dpr, 0) if __name__ == '__main__':