From b90473125964d3da239844e42a4c9fc82cd87725 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Thu, 4 Jan 2018 12:48:10 +1000 Subject: [PATCH] Respect transform context in point locator --- doc/api_break.dox | 3 ++- python/core/qgspointlocator.sip | 5 ++++- python/core/qgssnappingutils.sip | 5 +++++ src/core/qgspointlocator.cpp | 6 ++---- src/core/qgspointlocator.h | 9 ++++++--- src/core/qgssnappingutils.cpp | 4 ++-- src/core/qgssnappingutils.h | 6 +++--- src/gui/qgsmapcanvassnappingutils.cpp | 8 ++++++++ src/gui/qgsmapcanvassnappingutils.h | 1 + tests/src/core/testqgspointlocator.cpp | 8 ++++---- 10 files changed, 37 insertions(+), 18 deletions(-) diff --git a/doc/api_break.dox b/doc/api_break.dox index 58e196df5db6..0f0de7f0ed97 100644 --- a/doc/api_break.dox +++ b/doc/api_break.dox @@ -2002,7 +2002,8 @@ QgsPointLocator {#qgis_api_break_3_0_QgsPointLocator} --------------- - The constructor now takes a reference rather than a pointer to a CRS. This has no effect on PyQGIS code, but c++ -plugins calling this method will need to be updated. +plugins calling this method will need to be updated. The constructor now requires a QgsCoordinateTransformContext argument +when a destination crs is specified. - The destCRS parameter in the constructor has been renamed to destinationCrs. - destCRS() has been renamed to destinationCrs() - destinationCrs() now returns a copy instead of a reference to the CRS. This has no effect on PyQGIS code, but c++ diff --git a/python/core/qgspointlocator.sip b/python/core/qgspointlocator.sip index 447fb07f6d7d..9795d568f566 100644 --- a/python/core/qgspointlocator.sip +++ b/python/core/qgspointlocator.sip @@ -30,12 +30,15 @@ Works with one layer. public: explicit QgsPointLocator( QgsVectorLayer *layer, const QgsCoordinateReferenceSystem &destinationCrs = QgsCoordinateReferenceSystem(), + const QgsCoordinateTransformContext &transformContext = QgsCoordinateTransformContext(), const QgsRectangle *extent = 0 ); %Docstring Construct point locator for a ``layer``. If a valid QgsCoordinateReferenceSystem is passed for ``destinationCrs`` then the locator will -do the searches on data reprojected to the given CRS. +do the searches on data reprojected to the given CRS. For accurate reprojection it is important +to set the correct ``transformContext`` if a ``destinationCrs`` is specified. This is usually taken +from the current :py:func:`QgsProject.transformContext()` If ``extent`` is not null, the locator will index only a subset of the layer which falls within that extent. %End diff --git a/python/core/qgssnappingutils.sip b/python/core/qgssnappingutils.sip index 4c2c9b516fb1..9c168c7ff89f 100644 --- a/python/core/qgssnappingutils.sip +++ b/python/core/qgssnappingutils.sip @@ -181,6 +181,11 @@ Called when starting to index - can be overridden and e.g. progress dialog can b virtual void prepareIndexProgress( int index ); %Docstring Called when finished indexing a layer. When index == count the indexing is complete +%End + + void clearAllLocators(); +%Docstring +Deletes all existing locators (e.g. when destination CRS has changed and we need to reindex) %End }; diff --git a/src/core/qgspointlocator.cpp b/src/core/qgspointlocator.cpp index 5c8892c44250..418eeda0d5a8 100644 --- a/src/core/qgspointlocator.cpp +++ b/src/core/qgspointlocator.cpp @@ -620,15 +620,13 @@ class QgsPointLocator_DumpTree : public SpatialIndex::IQueryStrategy //////////////////////////////////////////////////////////////////////////// -QgsPointLocator::QgsPointLocator( QgsVectorLayer *layer, const QgsCoordinateReferenceSystem &destCRS, const QgsRectangle *extent ) +QgsPointLocator::QgsPointLocator( QgsVectorLayer *layer, const QgsCoordinateReferenceSystem &destCRS, const QgsCoordinateTransformContext &transformContext, const QgsRectangle *extent ) : mIsEmptyLayer( false ) , mLayer( layer ) { if ( destCRS.isValid() ) { - Q_NOWARN_DEPRECATED_PUSH - mTransform = QgsCoordinateTransform( layer->crs(), destCRS ); - Q_NOWARN_DEPRECATED_POP + mTransform = QgsCoordinateTransform( layer->crs(), destCRS, transformContext ); } setExtent( extent ); diff --git a/src/core/qgspointlocator.h b/src/core/qgspointlocator.h index a201bc386d72..c82757f55d81 100644 --- a/src/core/qgspointlocator.h +++ b/src/core/qgspointlocator.h @@ -55,12 +55,15 @@ class CORE_EXPORT QgsPointLocator : public QObject /** * Construct point locator for a \a layer. * - * If a valid QgsCoordinateReferenceSystem is passed for \a destinationCrs then the locator will - * do the searches on data reprojected to the given CRS. + * If a valid QgsCoordinateReferenceSystem is passed for \a destinationCrs then the locator will + * do the searches on data reprojected to the given CRS. For accurate reprojection it is important + * to set the correct \a transformContext if a \a destinationCrs is specified. This is usually taken + * from the current QgsProject::transformContext(). * - * If \a extent is not null, the locator will index only a subset of the layer which falls within that extent. + * If \a extent is not null, the locator will index only a subset of the layer which falls within that extent. */ explicit QgsPointLocator( QgsVectorLayer *layer, const QgsCoordinateReferenceSystem &destinationCrs = QgsCoordinateReferenceSystem(), + const QgsCoordinateTransformContext &transformContext = QgsCoordinateTransformContext(), const QgsRectangle *extent = nullptr ); ~QgsPointLocator() override; diff --git a/src/core/qgssnappingutils.cpp b/src/core/qgssnappingutils.cpp index 4dacfd572257..95b66b35925e 100644 --- a/src/core/qgssnappingutils.cpp +++ b/src/core/qgssnappingutils.cpp @@ -39,7 +39,7 @@ QgsPointLocator *QgsSnappingUtils::locatorForLayer( QgsVectorLayer *vl ) if ( !mLocators.contains( vl ) ) { - QgsPointLocator *vlpl = new QgsPointLocator( vl, destinationCrs() ); + QgsPointLocator *vlpl = new QgsPointLocator( vl, destinationCrs(), mMapSettings.transformContext() ); mLocators.insert( vl, vlpl ); } return mLocators.value( vl ); @@ -72,7 +72,7 @@ QgsPointLocator *QgsSnappingUtils::temporaryLocatorForLayer( QgsVectorLayer *vl, QgsRectangle rect( pointMap.x() - tolerance, pointMap.y() - tolerance, pointMap.x() + tolerance, pointMap.y() + tolerance ); - QgsPointLocator *vlpl = new QgsPointLocator( vl, destinationCrs(), &rect ); + QgsPointLocator *vlpl = new QgsPointLocator( vl, destinationCrs(), mMapSettings.transformContext(), &rect ); mTemporaryLocators.insert( vl, vlpl ); return mTemporaryLocators.value( vl ); } diff --git a/src/core/qgssnappingutils.h b/src/core/qgssnappingutils.h index 4f9bfa516b72..fbd70dc44c37 100644 --- a/src/core/qgssnappingutils.h +++ b/src/core/qgssnappingutils.h @@ -184,14 +184,14 @@ class CORE_EXPORT QgsSnappingUtils : public QObject //! Called when finished indexing a layer. When index == count the indexing is complete virtual void prepareIndexProgress( int index ) { Q_UNUSED( index ); } + //! Deletes all existing locators (e.g. when destination CRS has changed and we need to reindex) + void clearAllLocators(); + private: void onIndividualLayerSettingsChanged( const QHash &layerSettings ); //! Get destination CRS from map settings, or an invalid CRS if projections are disabled QgsCoordinateReferenceSystem destinationCrs() const; - //! delete all existing locators (e.g. when destination CRS has changed and we need to reindex) - void clearAllLocators(); - //! return a locator (temporary or not) according to the indexing strategy QgsPointLocator *locatorForLayerUsingStrategy( QgsVectorLayer *vl, const QgsPointXY &pointMap, double tolerance ); //! return a temporary locator with index only for a small area (will be replaced by another one on next request) diff --git a/src/gui/qgsmapcanvassnappingutils.cpp b/src/gui/qgsmapcanvassnappingutils.cpp index 753a2f550030..f1ce9b5f776a 100644 --- a/src/gui/qgsmapcanvassnappingutils.cpp +++ b/src/gui/qgsmapcanvassnappingutils.cpp @@ -29,6 +29,7 @@ QgsMapCanvasSnappingUtils::QgsMapCanvasSnappingUtils( QgsMapCanvas *canvas, QObj connect( canvas, &QgsMapCanvas::destinationCrsChanged, this, &QgsMapCanvasSnappingUtils::canvasMapSettingsChanged ); connect( canvas, &QgsMapCanvas::layersChanged, this, &QgsMapCanvasSnappingUtils::canvasMapSettingsChanged ); connect( canvas, &QgsMapCanvas::currentLayerChanged, this, &QgsMapCanvasSnappingUtils::canvasCurrentLayerChanged ); + connect( canvas, &QgsMapCanvas::transformContextChanged, this, &QgsMapCanvasSnappingUtils::canvasTransformContextChanged ); canvasMapSettingsChanged(); canvasCurrentLayerChanged(); } @@ -38,6 +39,13 @@ void QgsMapCanvasSnappingUtils::canvasMapSettingsChanged() setMapSettings( mCanvas->mapSettings() ); } +void QgsMapCanvasSnappingUtils::canvasTransformContextChanged() +{ + // can't trust any of our previous locators, as we don't know exactly how datum transform changes would affect these + clearAllLocators(); + setMapSettings( mCanvas->mapSettings() ); +} + void QgsMapCanvasSnappingUtils::canvasCurrentLayerChanged() { setCurrentLayer( qobject_cast( mCanvas->currentLayer() ) ); diff --git a/src/gui/qgsmapcanvassnappingutils.h b/src/gui/qgsmapcanvassnappingutils.h index d1cb6006cc74..39308530fc05 100644 --- a/src/gui/qgsmapcanvassnappingutils.h +++ b/src/gui/qgsmapcanvassnappingutils.h @@ -40,6 +40,7 @@ class GUI_EXPORT QgsMapCanvasSnappingUtils : public QgsSnappingUtils private slots: void canvasMapSettingsChanged(); + void canvasTransformContextChanged(); void canvasCurrentLayerChanged(); private: diff --git a/tests/src/core/testqgspointlocator.cpp b/tests/src/core/testqgspointlocator.cpp index 30b245f4d670..db074fb0ddd8 100644 --- a/tests/src/core/testqgspointlocator.cpp +++ b/tests/src/core/testqgspointlocator.cpp @@ -248,13 +248,13 @@ class TestQgsPointLocator : public QObject void testExtent() { QgsRectangle bbox1( 10, 10, 11, 11 ); // out of layer's bounds - QgsPointLocator loc1( mVL, QgsCoordinateReferenceSystem(), &bbox1 ); + QgsPointLocator loc1( mVL, QgsCoordinateReferenceSystem(), QgsCoordinateTransformContext(), &bbox1 ); QgsPointLocator::Match m1 = loc1.nearestVertex( QgsPointXY( 2, 2 ), 999 ); QVERIFY( !m1.isValid() ); QgsRectangle bbox2( 0, 0, 1, 1 ); // in layer's bounds - QgsPointLocator loc2( mVL, QgsCoordinateReferenceSystem(), &bbox2 ); + QgsPointLocator loc2( mVL, QgsCoordinateReferenceSystem(), QgsCoordinateTransformContext(), &bbox2 ); QgsPointLocator::Match m2 = loc2.nearestVertex( QgsPointXY( 2, 2 ), 999 ); QVERIFY( m2.isValid() ); @@ -270,7 +270,7 @@ class TestQgsPointLocator : public QObject flist << ff; vlNullGeom->dataProvider()->addFeatures( flist ); - QgsPointLocator loc( vlNullGeom, QgsCoordinateReferenceSystem(), nullptr ); + QgsPointLocator loc( vlNullGeom, QgsCoordinateReferenceSystem(), QgsCoordinateTransformContext(), nullptr ); QgsPointLocator::Match m1 = loc.nearestVertex( QgsPointXY( 2, 2 ), std::numeric_limits::max() ); QVERIFY( !m1.isValid() ); @@ -292,7 +292,7 @@ class TestQgsPointLocator : public QObject flist << ff; vlEmptyGeom->dataProvider()->addFeatures( flist ); - QgsPointLocator loc( vlEmptyGeom, QgsCoordinateReferenceSystem(), nullptr ); + QgsPointLocator loc( vlEmptyGeom, QgsCoordinateReferenceSystem(), QgsCoordinateTransformContext(), nullptr ); QgsPointLocator::Match m1 = loc.nearestVertex( QgsPointXY( 2, 2 ), std::numeric_limits::max() ); QVERIFY( !m1.isValid() );