Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Respect transform context in point locator
  • Loading branch information
nyalldawson committed Jan 4, 2018
1 parent 8f15cdf commit b904731
Show file tree
Hide file tree
Showing 10 changed files with 37 additions and 18 deletions.
3 changes: 2 additions & 1 deletion doc/api_break.dox
Expand Up @@ -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++
Expand Down
5 changes: 4 additions & 1 deletion python/core/qgspointlocator.sip
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions python/core/qgssnappingutils.sip
Expand Up @@ -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

};
Expand Down
6 changes: 2 additions & 4 deletions src/core/qgspointlocator.cpp
Expand Up @@ -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 );
Expand Down
9 changes: 6 additions & 3 deletions src/core/qgspointlocator.h
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/core/qgssnappingutils.cpp
Expand Up @@ -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 );
Expand Down Expand Up @@ -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 );
}
Expand Down
6 changes: 3 additions & 3 deletions src/core/qgssnappingutils.h
Expand Up @@ -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<QgsVectorLayer *, QgsSnappingConfig::IndividualLayerSettings> &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)
Expand Down
8 changes: 8 additions & 0 deletions src/gui/qgsmapcanvassnappingutils.cpp
Expand Up @@ -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();
}
Expand All @@ -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<QgsVectorLayer *>( mCanvas->currentLayer() ) );
Expand Down
1 change: 1 addition & 0 deletions src/gui/qgsmapcanvassnappingutils.h
Expand Up @@ -40,6 +40,7 @@ class GUI_EXPORT QgsMapCanvasSnappingUtils : public QgsSnappingUtils

private slots:
void canvasMapSettingsChanged();
void canvasTransformContextChanged();
void canvasCurrentLayerChanged();

private:
Expand Down
8 changes: 4 additions & 4 deletions tests/src/core/testqgspointlocator.cpp
Expand Up @@ -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() );
Expand All @@ -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<double>::max() );
QVERIFY( !m1.isValid() );
Expand All @@ -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<double>::max() );
QVERIFY( !m1.isValid() );
Expand Down

0 comments on commit b904731

Please sign in to comment.