From 003e5174ab83d020df8bdeac61eb38c36d741111 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Fri, 12 Jul 2024 12:25:41 +1000 Subject: [PATCH 1/9] Add equality operator for QgsCoordinateTransform --- .../proj/qgscoordinatetransform.sip.in | 3 + .../proj/qgscoordinatetransform.sip.in | 3 + src/core/proj/qgscoordinatetransform.cpp | 14 +++++ src/core/proj/qgscoordinatetransform.h | 3 + tests/src/core/testqgscoordinatetransform.cpp | 61 +++++++++++++++++++ 5 files changed, 84 insertions(+) diff --git a/python/PyQt6/core/auto_generated/proj/qgscoordinatetransform.sip.in b/python/PyQt6/core/auto_generated/proj/qgscoordinatetransform.sip.in index c56f092195ad..a0a95c91ff25 100644 --- a/python/PyQt6/core/auto_generated/proj/qgscoordinatetransform.sip.in +++ b/python/PyQt6/core/auto_generated/proj/qgscoordinatetransform.sip.in @@ -138,6 +138,9 @@ Copy constructor ~QgsCoordinateTransform(); + bool operator==( const QgsCoordinateTransform &other ) const; + bool operator!=( const QgsCoordinateTransform &other ) const; + static bool isTransformationPossible( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination ); %Docstring Returns ``True`` if it is theoretically possible to transform between ``source`` and ``destination`` CRSes. diff --git a/python/core/auto_generated/proj/qgscoordinatetransform.sip.in b/python/core/auto_generated/proj/qgscoordinatetransform.sip.in index c56f092195ad..a0a95c91ff25 100644 --- a/python/core/auto_generated/proj/qgscoordinatetransform.sip.in +++ b/python/core/auto_generated/proj/qgscoordinatetransform.sip.in @@ -138,6 +138,9 @@ Copy constructor ~QgsCoordinateTransform(); + bool operator==( const QgsCoordinateTransform &other ) const; + bool operator!=( const QgsCoordinateTransform &other ) const; + static bool isTransformationPossible( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination ); %Docstring Returns ``True`` if it is theoretically possible to transform between ``source`` and ``destination`` CRSes. diff --git a/src/core/proj/qgscoordinatetransform.cpp b/src/core/proj/qgscoordinatetransform.cpp index 09618c4109e7..caab2b3623c0 100644 --- a/src/core/proj/qgscoordinatetransform.cpp +++ b/src/core/proj/qgscoordinatetransform.cpp @@ -174,6 +174,20 @@ QgsCoordinateTransform &QgsCoordinateTransform::operator=( const QgsCoordinateTr QgsCoordinateTransform::~QgsCoordinateTransform() {} //NOLINT +bool QgsCoordinateTransform::operator==( const QgsCoordinateTransform &other ) const +{ + return d->mSourceCRS == other.d->mSourceCRS + && d->mDestCRS == other.d->mDestCRS + && mBallparkTransformsAreAppropriate == other.mBallparkTransformsAreAppropriate + && d->mProjCoordinateOperation == other.d->mProjCoordinateOperation + && instantiatedCoordinateOperationDetails().proj == other.instantiatedCoordinateOperationDetails().proj; +} + +bool QgsCoordinateTransform::operator!=( const QgsCoordinateTransform &other ) const +{ + return !( *this == other ); +} + bool QgsCoordinateTransform::isTransformationPossible( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination ) { if ( !source.isValid() || !destination.isValid() ) diff --git a/src/core/proj/qgscoordinatetransform.h b/src/core/proj/qgscoordinatetransform.h index 975b9e18e774..2048d37dfb47 100644 --- a/src/core/proj/qgscoordinatetransform.h +++ b/src/core/proj/qgscoordinatetransform.h @@ -147,6 +147,9 @@ class CORE_EXPORT QgsCoordinateTransform ~QgsCoordinateTransform(); + bool operator==( const QgsCoordinateTransform &other ) const; + bool operator!=( const QgsCoordinateTransform &other ) const; + /** * Returns TRUE if it is theoretically possible to transform between \a source and \a destination CRSes. * diff --git a/tests/src/core/testqgscoordinatetransform.cpp b/tests/src/core/testqgscoordinatetransform.cpp index a3bd1c68ff1e..9b7ba58fb9f0 100644 --- a/tests/src/core/testqgscoordinatetransform.cpp +++ b/tests/src/core/testqgscoordinatetransform.cpp @@ -34,6 +34,7 @@ class TestQgsCoordinateTransform: public QObject void transformBoundingBox(); void copy(); void assignment(); + void equality(); void isValid(); void isShortCircuited(); void contextShared(); @@ -125,6 +126,66 @@ void TestQgsCoordinateTransform::assignment() QVERIFY( original.isValid() ); } +void TestQgsCoordinateTransform::equality() +{ + QgsCoordinateTransform t1; + QgsCoordinateTransform t2; + QVERIFY( t1 == t2 ); + + t1 = QgsCoordinateTransform( QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3111" ) ), + QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3857" ) ), QgsCoordinateTransformContext() ); + QVERIFY( t1 != t2 ); + QVERIFY( t2 != t1 ); + // same source and destination + t2 = QgsCoordinateTransform( QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3111" ) ), + QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3857" ) ), QgsCoordinateTransformContext() ); + QVERIFY( t1 == t2 ); + QVERIFY( t2 == t1 ); + // different source + t2 = QgsCoordinateTransform( QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3113" ) ), + QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3857" ) ), QgsCoordinateTransformContext() ); + QVERIFY( t1 != t2 ); + QVERIFY( t2 != t1 ); + // different destination + t2 = QgsCoordinateTransform( QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3111" ) ), + QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:4326" ) ), QgsCoordinateTransformContext() ); + QVERIFY( t1 != t2 ); + QVERIFY( t2 != t1 ); + // different source and destination + t2 = QgsCoordinateTransform( QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3113" ) ), + QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:4326" ) ), QgsCoordinateTransformContext() ); + QVERIFY( t1 != t2 ); + QVERIFY( t2 != t1 ); + // source/destination swapped + t2 = QgsCoordinateTransform( QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3857" ) ), + QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3111" ) ), QgsCoordinateTransformContext() ); + QVERIFY( t1 != t2 ); + QVERIFY( t2 != t1 ); + + // same source and dest, different settings + t2 = QgsCoordinateTransform( QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3111" ) ), + QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3857" ) ), QgsCoordinateTransformContext() ); + t1.setBallparkTransformsAreAppropriate( true ); + QVERIFY( t1 != t2 ); + QVERIFY( t2 != t1 ); + t2.setBallparkTransformsAreAppropriate( true ); + QVERIFY( t1 == t2 ); + + // explicit coordinate operation + t1 = QgsCoordinateTransform( QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3111" ) ), + QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3857" ) ), QgsCoordinateTransformContext() ); + t1.setCoordinateOperation( QStringLiteral( "+proj=pipeline +step +inv +proj=lcc +lat_0=-37 +lon_0=145 +lat_1=-36 +lat_2=-38 +x_0=2500000 +y_0=2500000 +ellps=GRS80 +step +proj=hgridshift +grids=au_icsm_GDA94_GDA2020_conformal_and_distortion.tif +step +proj=webmerc +lat_0=0 +lon_0=0 +x_0=0 +y_0=0 +ellps=WGS84" ) ); + t2 = QgsCoordinateTransform( QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3111" ) ), + QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3857" ) ), QgsCoordinateTransformContext() ); + QVERIFY( t1 != t2 ); + QVERIFY( t2 != t1 ); + t2.setCoordinateOperation( QStringLiteral( "+proj=pipeline +step +inv +proj=lcc +lat_0=-37 +lon_0=145 +lat_1=-36 +lat_2=-38 +x_0=2500000 +y_0=2500000 +ellps=GRS80 +step +proj=webmerc +lat_0=0 +lon_0=0 +x_0=0 +y_0=0 +ellps=WGS84" ) ); + QVERIFY( t1 != t2 ); + QVERIFY( t2 != t1 ); + t2.setCoordinateOperation( t1.coordinateOperation() ); + QVERIFY( t1 == t2 ); +} + void TestQgsCoordinateTransform::isValid() { const QgsCoordinateTransform tr; From 1f9aab819f08ff806058530effde41982771c67a Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Fri, 12 Jul 2024 12:51:22 +1000 Subject: [PATCH 2/9] Add QgsCoordinateTransform setter for QgsFeatureRequest Sets the coordinate transform which will be used to transform the feature's geometries. If this transform is valid then it will always be used to transform features, regardless of the destinationCrs() setting or the underlying feature source's actual CRS. This method should be used with caution, and it is recommended to use the high-level setDestinationCrs() method instead. Setting a specific transform should only be done when there is a requirement to use a particular transform, eg when a specific, non-default coordinate operation MUST be used for the transformation. --- .../auto_generated/qgsfeaturerequest.sip.in | 64 ++++++++++++++++++- .../auto_generated/qgsfeaturerequest.sip.in | 64 ++++++++++++++++++- src/core/qgsfeaturerequest.cpp | 13 +++- src/core/qgsfeaturerequest.h | 59 ++++++++++++++++- tests/src/python/test_qgsfeaturerequest.py | 40 +++++++++++- 5 files changed, 235 insertions(+), 5 deletions(-) diff --git a/python/PyQt6/core/auto_generated/qgsfeaturerequest.sip.in b/python/PyQt6/core/auto_generated/qgsfeaturerequest.sip.in index b38b3c3316c1..95f4b3c8be64 100644 --- a/python/PyQt6/core/auto_generated/qgsfeaturerequest.sip.in +++ b/python/PyQt6/core/auto_generated/qgsfeaturerequest.sip.in @@ -8,7 +8,6 @@ - class QgsFeatureRequest { %Docstring(signature="appended") @@ -714,6 +713,20 @@ Set a simplification method for geometries that will be fetched. Returns the simplification method for geometries that will be fetched. .. seealso:: :py:func:`setSimplifyMethod` +%End + + QgsCoordinateTransform coordinateTransform() const; +%Docstring +Returns the coordinate transform which will be used to transform +the feature's geometries. + +If this transform is valid then it will always be used to transform +features, regardless of the :py:func:`~QgsFeatureRequest.destinationCrs` setting or the underlying +feature source's actual CRS. + +.. seealso:: :py:func:`setCoordinateTransform` + +.. versionadded:: 3.40 %End QgsCoordinateReferenceSystem destinationCrs() const; @@ -722,6 +735,14 @@ Returns the destination coordinate reference system for feature's geometries, or an invalid :py:class:`QgsCoordinateReferenceSystem` if no reprojection will be done and all features will be left with their original geometry. +.. warning:: + + if :py:func:`~QgsFeatureRequest.coordinateTransform` returns a valid transform then the + :py:func:`~QgsFeatureRequest.destinationCrs` will have no effect, and the :py:func:`~QgsFeatureRequest.coordinateTransform` will + always be used to transform features. + +.. seealso:: :py:func:`calculateTransform` + .. seealso:: :py:func:`setDestinationCrs` .. seealso:: :py:func:`transformContext` @@ -735,6 +756,41 @@ and reprojection is required .. seealso:: :py:func:`setDestinationCrs` .. seealso:: :py:func:`destinationCrs` +%End + + QgsFeatureRequest &setCoordinateTransform( const QgsCoordinateTransform &transform ); +%Docstring +Sets the coordinate ``transform`` which will be used to transform +the feature's geometries. + +If this transform is valid then it will always be used to transform +features, regardless of the :py:func:`~QgsFeatureRequest.destinationCrs` setting or the underlying +feature source's actual CRS. + +When a ``transform`` is set using :py:func:`~QgsFeatureRequest.setCoordinateTransform`, then any :py:func:`~QgsFeatureRequest.filterRect` +or :py:func:`~QgsFeatureRequest.referenceGeometry` set on the request is expected to be in the +same CRS as the destination CRS for the ``transform``. + +The feature geometry transformation is performed +after all filter expressions are tested and any virtual fields are +calculated. Accordingly, any geometric expressions used in +:py:func:`~QgsFeatureRequest.filterExpression` will be performed in the original +source CRS. This ensures consistent results are returned regardless of the +destination CRS. Similarly, virtual field values will be calculated using the +original geometry in the source CRS, so these values are not affected by +any destination CRS transform present in the feature request. + +.. warning:: + + This method should be used with caution, and it is recommended + to use the high-level :py:func:`~QgsFeatureRequest.setDestinationCrs` method instead. Setting a specific + transform should only be done when there is a requirement to use a particular + transform. + +.. seealso:: :py:func:`coordinateTransform` + + +.. versionadded:: 3.40 %End QgsFeatureRequest &setDestinationCrs( const QgsCoordinateReferenceSystem &crs, const QgsCoordinateTransformContext &context ); @@ -758,6 +814,12 @@ destination CRS. Similarly, virtual field values will be calculated using the original geometry in the source CRS, so these values are not affected by any destination CRS transform present in the feature request. +.. warning:: + + if :py:func:`~QgsFeatureRequest.coordinateTransform` returns a valid transform then the + :py:func:`~QgsFeatureRequest.destinationCrs` will have no effect, and the :py:func:`~QgsFeatureRequest.coordinateTransform` will + always be used to transform features. + .. seealso:: :py:func:`destinationCrs` %End diff --git a/python/core/auto_generated/qgsfeaturerequest.sip.in b/python/core/auto_generated/qgsfeaturerequest.sip.in index b38b3c3316c1..95f4b3c8be64 100644 --- a/python/core/auto_generated/qgsfeaturerequest.sip.in +++ b/python/core/auto_generated/qgsfeaturerequest.sip.in @@ -8,7 +8,6 @@ - class QgsFeatureRequest { %Docstring(signature="appended") @@ -714,6 +713,20 @@ Set a simplification method for geometries that will be fetched. Returns the simplification method for geometries that will be fetched. .. seealso:: :py:func:`setSimplifyMethod` +%End + + QgsCoordinateTransform coordinateTransform() const; +%Docstring +Returns the coordinate transform which will be used to transform +the feature's geometries. + +If this transform is valid then it will always be used to transform +features, regardless of the :py:func:`~QgsFeatureRequest.destinationCrs` setting or the underlying +feature source's actual CRS. + +.. seealso:: :py:func:`setCoordinateTransform` + +.. versionadded:: 3.40 %End QgsCoordinateReferenceSystem destinationCrs() const; @@ -722,6 +735,14 @@ Returns the destination coordinate reference system for feature's geometries, or an invalid :py:class:`QgsCoordinateReferenceSystem` if no reprojection will be done and all features will be left with their original geometry. +.. warning:: + + if :py:func:`~QgsFeatureRequest.coordinateTransform` returns a valid transform then the + :py:func:`~QgsFeatureRequest.destinationCrs` will have no effect, and the :py:func:`~QgsFeatureRequest.coordinateTransform` will + always be used to transform features. + +.. seealso:: :py:func:`calculateTransform` + .. seealso:: :py:func:`setDestinationCrs` .. seealso:: :py:func:`transformContext` @@ -735,6 +756,41 @@ and reprojection is required .. seealso:: :py:func:`setDestinationCrs` .. seealso:: :py:func:`destinationCrs` +%End + + QgsFeatureRequest &setCoordinateTransform( const QgsCoordinateTransform &transform ); +%Docstring +Sets the coordinate ``transform`` which will be used to transform +the feature's geometries. + +If this transform is valid then it will always be used to transform +features, regardless of the :py:func:`~QgsFeatureRequest.destinationCrs` setting or the underlying +feature source's actual CRS. + +When a ``transform`` is set using :py:func:`~QgsFeatureRequest.setCoordinateTransform`, then any :py:func:`~QgsFeatureRequest.filterRect` +or :py:func:`~QgsFeatureRequest.referenceGeometry` set on the request is expected to be in the +same CRS as the destination CRS for the ``transform``. + +The feature geometry transformation is performed +after all filter expressions are tested and any virtual fields are +calculated. Accordingly, any geometric expressions used in +:py:func:`~QgsFeatureRequest.filterExpression` will be performed in the original +source CRS. This ensures consistent results are returned regardless of the +destination CRS. Similarly, virtual field values will be calculated using the +original geometry in the source CRS, so these values are not affected by +any destination CRS transform present in the feature request. + +.. warning:: + + This method should be used with caution, and it is recommended + to use the high-level :py:func:`~QgsFeatureRequest.setDestinationCrs` method instead. Setting a specific + transform should only be done when there is a requirement to use a particular + transform. + +.. seealso:: :py:func:`coordinateTransform` + + +.. versionadded:: 3.40 %End QgsFeatureRequest &setDestinationCrs( const QgsCoordinateReferenceSystem &crs, const QgsCoordinateTransformContext &context ); @@ -758,6 +814,12 @@ destination CRS. Similarly, virtual field values will be calculated using the original geometry in the source CRS, so these values are not affected by any destination CRS transform present in the feature request. +.. warning:: + + if :py:func:`~QgsFeatureRequest.coordinateTransform` returns a valid transform then the + :py:func:`~QgsFeatureRequest.destinationCrs` will have no effect, and the :py:func:`~QgsFeatureRequest.coordinateTransform` will + always be used to transform features. + .. seealso:: :py:func:`destinationCrs` %End diff --git a/src/core/qgsfeaturerequest.cpp b/src/core/qgsfeaturerequest.cpp index 38b8f4bddfeb..7df7cb8b130b 100644 --- a/src/core/qgsfeaturerequest.cpp +++ b/src/core/qgsfeaturerequest.cpp @@ -89,6 +89,7 @@ QgsFeatureRequest &QgsFeatureRequest::operator=( const QgsFeatureRequest &rh ) mSimplifyMethod = rh.mSimplifyMethod; mLimit = rh.mLimit; mOrderBy = rh.mOrderBy; + mTransform = rh.mTransform; mCrs = rh.mCrs; mTransformContext = rh.mTransformContext; mTransformErrorCallback = rh.mTransformErrorCallback; @@ -118,11 +119,11 @@ bool QgsFeatureRequest::compare( const QgsFeatureRequest &rh ) const mSimplifyMethod == rh.mSimplifyMethod && mLimit == rh.mLimit && mOrderBy == rh.mOrderBy && + mTransform == rh.mTransform && mCrs == rh.mCrs && mTransformContext == rh.mTransformContext && mTimeout == rh.mTimeout && mRequestMayBeNested == rh.mRequestMayBeNested; - } @@ -315,6 +316,10 @@ QgsFeatureRequest &QgsFeatureRequest::setSimplifyMethod( const QgsSimplifyMethod return *this; } +QgsCoordinateTransform QgsFeatureRequest::coordinateTransform() const +{ + return mTransform; +} QgsCoordinateReferenceSystem QgsFeatureRequest::destinationCrs() const { @@ -326,6 +331,12 @@ QgsCoordinateTransformContext QgsFeatureRequest::transformContext() const return mTransformContext; } +QgsFeatureRequest &QgsFeatureRequest::setCoordinateTransform( const QgsCoordinateTransform &transform ) +{ + mTransform = transform; + return *this; +} + QgsFeatureRequest &QgsFeatureRequest::setDestinationCrs( const QgsCoordinateReferenceSystem &crs, const QgsCoordinateTransformContext &context ) { mCrs = crs; diff --git a/src/core/qgsfeaturerequest.h b/src/core/qgsfeaturerequest.h index 2cea770c2d98..33d2c6ebf32a 100644 --- a/src/core/qgsfeaturerequest.h +++ b/src/core/qgsfeaturerequest.h @@ -29,7 +29,7 @@ #include "qgssimplifymethod.h" #include "qgscoordinatetransformcontext.h" #include "qgscoordinatereferencesystem.h" - +#include "qgscoordinatetransform.h" /** * \ingroup core @@ -723,10 +723,30 @@ class CORE_EXPORT QgsFeatureRequest */ const QgsSimplifyMethod &simplifyMethod() const { return mSimplifyMethod; } + /** + * Returns the coordinate transform which will be used to transform + * the feature's geometries. + * + * If this transform is valid then it will always be used to transform + * features, regardless of the destinationCrs() setting or the underlying + * feature source's actual CRS. + * + * \see setCoordinateTransform() + * + * \since QGIS 3.40 + */ + QgsCoordinateTransform coordinateTransform() const; + /** * Returns the destination coordinate reference system for feature's geometries, * or an invalid QgsCoordinateReferenceSystem if no reprojection will be done * and all features will be left with their original geometry. + * + * \warning if coordinateTransform() returns a valid transform then the + * destinationCrs() will have no effect, and the coordinateTransform() will + * always be used to transform features. + * + * \see calculateTransform() * \see setDestinationCrs() * \see transformContext() */ @@ -740,6 +760,38 @@ class CORE_EXPORT QgsFeatureRequest */ QgsCoordinateTransformContext transformContext() const; + /** + * Sets the coordinate \a transform which will be used to transform + * the feature's geometries. + * + * If this transform is valid then it will always be used to transform + * features, regardless of the destinationCrs() setting or the underlying + * feature source's actual CRS. + * + * When a \a transform is set using setCoordinateTransform(), then any filterRect() + * or referenceGeometry() set on the request is expected to be in the + * same CRS as the destination CRS for the \a transform. + * + * The feature geometry transformation is performed + * after all filter expressions are tested and any virtual fields are + * calculated. Accordingly, any geometric expressions used in + * filterExpression() will be performed in the original + * source CRS. This ensures consistent results are returned regardless of the + * destination CRS. Similarly, virtual field values will be calculated using the + * original geometry in the source CRS, so these values are not affected by + * any destination CRS transform present in the feature request. + * + * \warning This method should be used with caution, and it is recommended + * to use the high-level setDestinationCrs() method instead. Setting a specific + * transform should only be done when there is a requirement to use a particular + * transform. + * + * \see coordinateTransform() + * + * \since QGIS 3.40 + */ + QgsFeatureRequest &setCoordinateTransform( const QgsCoordinateTransform &transform ); + /** * Sets the destination \a crs for feature's geometries. If set, all * geometries will be reprojected from their original coordinate reference @@ -760,6 +812,10 @@ class CORE_EXPORT QgsFeatureRequest * original geometry in the source CRS, so these values are not affected by * any destination CRS transform present in the feature request. * + * \warning if coordinateTransform() returns a valid transform then the + * destinationCrs() will have no effect, and the coordinateTransform() will + * always be used to transform features. + * * \see destinationCrs() */ QgsFeatureRequest &setDestinationCrs( const QgsCoordinateReferenceSystem &crs, const QgsCoordinateTransformContext &context ); @@ -952,6 +1008,7 @@ class CORE_EXPORT QgsFeatureRequest Qgis::InvalidGeometryCheck mInvalidGeometryFilter = Qgis::InvalidGeometryCheck::NoCheck; std::function< void( const QgsFeature & ) > mInvalidGeometryCallback; std::function< void( const QgsFeature & ) > mTransformErrorCallback; + QgsCoordinateTransform mTransform; QgsCoordinateReferenceSystem mCrs; QgsCoordinateTransformContext mTransformContext; int mTimeout = -1; diff --git a/tests/src/python/test_qgsfeaturerequest.py b/tests/src/python/test_qgsfeaturerequest.py index 2f1a066b5350..3fbcd21eea48 100644 --- a/tests/src/python/test_qgsfeaturerequest.py +++ b/tests/src/python/test_qgsfeaturerequest.py @@ -22,6 +22,7 @@ QgsGeometry, QgsRectangle, QgsSimplifyMethod, + QgsCoordinateTransform ) import unittest from qgis.testing import start_app, QgisTestCase @@ -373,6 +374,20 @@ def testNested(self): req.setRequestMayBeNested(True) self.assertTrue(req.requestMayBeNested()) + def testCoordinateTransform(self): + req = QgsFeatureRequest() + self.assertFalse(req.coordinateTransform().isValid()) + req.setCoordinateTransform( + QgsCoordinateTransform( + QgsCoordinateReferenceSystem('EPSG:3111'), + QgsCoordinateReferenceSystem('EPSG:3857'), + QgsCoordinateTransformContext() + ) + ) + self.assertTrue(req.coordinateTransform().isValid()) + self.assertEqual(req.coordinateTransform().sourceCrs().authid(), 'EPSG:3111') + self.assertEqual(req.coordinateTransform().destinationCrs().authid(), 'EPSG:3857') + def testAssignment(self): req = QgsFeatureRequest().setFilterFids([8, 9]).setFilterRect(QgsRectangle(1, 2, 3, 4)).setInvalidGeometryCheck(QgsFeatureRequest.InvalidGeometryCheck.GeometrySkipInvalid).setLimit(6).setFlags(QgsFeatureRequest.Flag.ExactIntersect).setSubsetOfAttributes([1, 4]).setTimeout(6).setRequestMayBeNested(True) @@ -386,6 +401,13 @@ def testAssignment(self): req.setSimplifyMethod(method) context = QgsCoordinateTransformContext() req.setDestinationCrs(QgsCoordinateReferenceSystem('EPSG:3857'), context) + req.setCoordinateTransform( + QgsCoordinateTransform( + QgsCoordinateReferenceSystem('EPSG:3111'), + QgsCoordinateReferenceSystem('EPSG:3857'), + QgsCoordinateTransformContext() + ) + ) req2 = QgsFeatureRequest(req) self.assertEqual(req2.limit(), 6) @@ -401,6 +423,8 @@ def testAssignment(self): self.assertEqual(req2.destinationCrs().authid(), 'EPSG:3857') self.assertEqual(req2.timeout(), 6) self.assertTrue(req2.requestMayBeNested()) + self.assertEqual(req2.coordinateTransform().sourceCrs().authid(), 'EPSG:3111') + self.assertEqual(req2.coordinateTransform().destinationCrs().authid(), 'EPSG:3857') # copy distance within request req = QgsFeatureRequest().setDistanceWithin(QgsGeometry.fromWkt('LineString( 0 0, 10 0, 11 2)'), 1.2) @@ -411,7 +435,6 @@ def testAssignment(self): self.assertEqual(req2.filterRect(), QgsRectangle(-1.2, -1.2, 12.2, 3.2)) def test_compare(self): - req1 = QgsFeatureRequest().setFilterFids([8, 9]).setFilterRect(QgsRectangle(1, 2, 3, 4)).setInvalidGeometryCheck(QgsFeatureRequest.InvalidGeometryCheck.GeometrySkipInvalid).setLimit(6).setFlags(QgsFeatureRequest.Flag.ExactIntersect).setSubsetOfAttributes([1, 4]).setTimeout(6).setRequestMayBeNested(True) req2 = QgsFeatureRequest(req1) self.assertTrue(req1.compare(req1)) @@ -472,6 +495,21 @@ def test_compare(self): req3.setExpressionContext(context) self.assertTrue(req3.compare(req1)) + # coordinate transform + req3 = QgsFeatureRequest(req2) + req2.setCoordinateTransform( + QgsCoordinateTransform( + QgsCoordinateReferenceSystem('EPSG:3111'), + QgsCoordinateReferenceSystem('EPSG:3857'), + QgsCoordinateTransformContext() + ) + ) + self.assertFalse(req3.compare(req2)) + req3.setCoordinateTransform( + req2.coordinateTransform() + ) + self.assertTrue(req3.compare(req2)) + def test_order_by_equality(self): orderClause1 = QgsFeatureRequest.OrderByClause('a', False) From 9c58f69ba5c56242671e3afe5a0183043a94d55c Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Fri, 12 Jul 2024 12:58:10 +1000 Subject: [PATCH 3/9] Add helper method to calculate transform to apply for a QgsFeatureRequest --- .../auto_generated/qgsfeaturerequest.sip.in | 11 ++++++ .../auto_generated/qgsfeaturerequest.sip.in | 11 ++++++ src/core/qgsfeaturerequest.cpp | 13 +++++++ src/core/qgsfeaturerequest.h | 11 ++++++ tests/src/python/test_qgsfeaturerequest.py | 37 +++++++++++++++++-- 5 files changed, 80 insertions(+), 3 deletions(-) diff --git a/python/PyQt6/core/auto_generated/qgsfeaturerequest.sip.in b/python/PyQt6/core/auto_generated/qgsfeaturerequest.sip.in index 95f4b3c8be64..2bebc26aabcf 100644 --- a/python/PyQt6/core/auto_generated/qgsfeaturerequest.sip.in +++ b/python/PyQt6/core/auto_generated/qgsfeaturerequest.sip.in @@ -756,6 +756,17 @@ and reprojection is required .. seealso:: :py:func:`setDestinationCrs` .. seealso:: :py:func:`destinationCrs` +%End + + QgsCoordinateTransform calculateTransform( const QgsCoordinateReferenceSystem &sourceCrs ) const; +%Docstring +Calculates the coordinate transform to use to transform geometries +when they are originally in ``sourceCrs``. + +This method will return :py:func:`~QgsFeatureRequest.coordinateTransform` if it is set (ignoring ``sourceCrs``), otherwise +it will calculate an appriopriate transform from ``sourceCrs`` to :py:func:`~QgsFeatureRequest.destinationCrs`. + +.. versionadded:: 3.40 %End QgsFeatureRequest &setCoordinateTransform( const QgsCoordinateTransform &transform ); diff --git a/python/core/auto_generated/qgsfeaturerequest.sip.in b/python/core/auto_generated/qgsfeaturerequest.sip.in index 95f4b3c8be64..2bebc26aabcf 100644 --- a/python/core/auto_generated/qgsfeaturerequest.sip.in +++ b/python/core/auto_generated/qgsfeaturerequest.sip.in @@ -756,6 +756,17 @@ and reprojection is required .. seealso:: :py:func:`setDestinationCrs` .. seealso:: :py:func:`destinationCrs` +%End + + QgsCoordinateTransform calculateTransform( const QgsCoordinateReferenceSystem &sourceCrs ) const; +%Docstring +Calculates the coordinate transform to use to transform geometries +when they are originally in ``sourceCrs``. + +This method will return :py:func:`~QgsFeatureRequest.coordinateTransform` if it is set (ignoring ``sourceCrs``), otherwise +it will calculate an appriopriate transform from ``sourceCrs`` to :py:func:`~QgsFeatureRequest.destinationCrs`. + +.. versionadded:: 3.40 %End QgsFeatureRequest &setCoordinateTransform( const QgsCoordinateTransform &transform ); diff --git a/src/core/qgsfeaturerequest.cpp b/src/core/qgsfeaturerequest.cpp index 7df7cb8b130b..9078fa24beb0 100644 --- a/src/core/qgsfeaturerequest.cpp +++ b/src/core/qgsfeaturerequest.cpp @@ -331,6 +331,19 @@ QgsCoordinateTransformContext QgsFeatureRequest::transformContext() const return mTransformContext; } +QgsCoordinateTransform QgsFeatureRequest::calculateTransform( const QgsCoordinateReferenceSystem &sourceCrs ) const +{ + if ( mTransform.isValid() ) + { + return mTransform; + } + else if ( sourceCrs.isValid() && mCrs != sourceCrs ) + { + return QgsCoordinateTransform( sourceCrs, mCrs, mTransformContext ); + } + return QgsCoordinateTransform(); +} + QgsFeatureRequest &QgsFeatureRequest::setCoordinateTransform( const QgsCoordinateTransform &transform ) { mTransform = transform; diff --git a/src/core/qgsfeaturerequest.h b/src/core/qgsfeaturerequest.h index 33d2c6ebf32a..aa48d7af2cd7 100644 --- a/src/core/qgsfeaturerequest.h +++ b/src/core/qgsfeaturerequest.h @@ -760,6 +760,17 @@ class CORE_EXPORT QgsFeatureRequest */ QgsCoordinateTransformContext transformContext() const; + /** + * Calculates the coordinate transform to use to transform geometries + * when they are originally in \a sourceCrs. + * + * This method will return coordinateTransform() if it is set (ignoring \a sourceCrs), otherwise + * it will calculate an appriopriate transform from \a sourceCrs to destinationCrs(). + * + * \since QGIS 3.40 + */ + QgsCoordinateTransform calculateTransform( const QgsCoordinateReferenceSystem &sourceCrs ) const; + /** * Sets the coordinate \a transform which will be used to transform * the feature's geometries. diff --git a/tests/src/python/test_qgsfeaturerequest.py b/tests/src/python/test_qgsfeaturerequest.py index 3fbcd21eea48..33660906dd17 100644 --- a/tests/src/python/test_qgsfeaturerequest.py +++ b/tests/src/python/test_qgsfeaturerequest.py @@ -381,7 +381,7 @@ def testCoordinateTransform(self): QgsCoordinateTransform( QgsCoordinateReferenceSystem('EPSG:3111'), QgsCoordinateReferenceSystem('EPSG:3857'), - QgsCoordinateTransformContext() + QgsCoordinateTransformContext() ) ) self.assertTrue(req.coordinateTransform().isValid()) @@ -405,7 +405,7 @@ def testAssignment(self): QgsCoordinateTransform( QgsCoordinateReferenceSystem('EPSG:3111'), QgsCoordinateReferenceSystem('EPSG:3857'), - QgsCoordinateTransformContext() + QgsCoordinateTransformContext() ) ) @@ -501,7 +501,7 @@ def test_compare(self): QgsCoordinateTransform( QgsCoordinateReferenceSystem('EPSG:3111'), QgsCoordinateReferenceSystem('EPSG:3857'), - QgsCoordinateTransformContext() + QgsCoordinateTransformContext() ) ) self.assertFalse(req3.compare(req2)) @@ -528,6 +528,37 @@ def test_order_by_equality(self): order2 = QgsFeatureRequest.OrderBy([orderClause1, orderClause2]) self.assertFalse(order1 == order2) + def test_calculate_transform(self): + """ + Test transform calculation + """ + req = QgsFeatureRequest() + # no transformation + transform = req.calculateTransform(QgsCoordinateReferenceSystem('EPSG:4326')) + self.assertFalse(transform.isValid()) + + # transform using destination crs + req.setDestinationCrs(QgsCoordinateReferenceSystem('EPSG:3857'), + QgsCoordinateTransformContext()) + transform = req.calculateTransform(QgsCoordinateReferenceSystem('EPSG:4326')) + self.assertTrue(transform.isValid()) + self.assertEqual(transform.sourceCrs().authid(), 'EPSG:4326') + self.assertEqual(transform.destinationCrs().authid(), 'EPSG:3857') + + # transform using a specific coordinate transform, must take precedence + req.setCoordinateTransform( + QgsCoordinateTransform( + QgsCoordinateReferenceSystem('EPSG:3111'), + QgsCoordinateReferenceSystem('EPSG:3857'), + QgsCoordinateTransformContext() + ) + ) + # source crs is ignored + transform = req.calculateTransform(QgsCoordinateReferenceSystem('EPSG:4326')) + self.assertTrue(transform.isValid()) + self.assertEqual(transform.sourceCrs().authid(), 'EPSG:3111') + self.assertEqual(transform.destinationCrs().authid(), 'EPSG:3857') + if __name__ == '__main__': unittest.main() From 1e11a6d56790504e5a3be713019568ea55053ce6 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Fri, 12 Jul 2024 13:01:07 +1000 Subject: [PATCH 4/9] Respect transform from feature request in iterator implementations --- .../memory/qgsmemoryfeatureiterator.cpp | 6 +- .../providers/ogr/qgsogrfeatureiterator.cpp | 5 +- .../qgssensorthingsfeatureiterator.cpp | 5 +- src/core/qgscachedfeatureiterator.cpp | 10 +-- src/core/qgsfeaturesource.cpp | 3 +- .../vector/qgsvectorlayerfeatureiterator.cpp | 10 ++- .../arcgisrest/qgsafsfeatureiterator.cpp | 5 +- .../qgsdelimitedtextfeatureiterator.cpp | 5 +- src/providers/gpx/qgsgpxfeatureiterator.cpp | 5 +- src/providers/hana/qgshanafeatureiterator.cpp | 3 +- .../mssql/qgsmssqlfeatureiterator.cpp | 5 +- .../oracle/qgsoraclefeatureiterator.cpp | 5 +- .../postgres/qgspostgresfeatureiterator.cpp | 5 +- .../qgsspatialitefeatureiterator.cpp | 5 +- .../qgsvirtuallayerfeatureiterator.cpp | 5 +- .../qgsbackgroundcachedfeatureiterator.cpp | 5 +- tests/src/python/featuresourcetestbase.py | 62 ++++++++++++++++++- 17 files changed, 82 insertions(+), 67 deletions(-) diff --git a/src/core/providers/memory/qgsmemoryfeatureiterator.cpp b/src/core/providers/memory/qgsmemoryfeatureiterator.cpp index c6482e629457..05b1e4b59ca2 100644 --- a/src/core/providers/memory/qgsmemoryfeatureiterator.cpp +++ b/src/core/providers/memory/qgsmemoryfeatureiterator.cpp @@ -28,10 +28,8 @@ QgsMemoryFeatureIterator::QgsMemoryFeatureIterator( QgsMemoryFeatureSource *source, bool ownSource, const QgsFeatureRequest &request ) : QgsAbstractFeatureIteratorFromSource( source, ownSource, request ) { - if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mSource->mCrs ) - { - mTransform = QgsCoordinateTransform( mSource->mCrs, mRequest.destinationCrs(), mRequest.transformContext() ); - } + mTransform = mRequest.calculateTransform( mSource->mCrs ); + try { mFilterRect = filterRectToSourceCrs( mTransform ); diff --git a/src/core/providers/ogr/qgsogrfeatureiterator.cpp b/src/core/providers/ogr/qgsogrfeatureiterator.cpp index 86e57024df17..90fcabef882d 100644 --- a/src/core/providers/ogr/qgsogrfeatureiterator.cpp +++ b/src/core/providers/ogr/qgsogrfeatureiterator.cpp @@ -123,10 +123,7 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool } QMutexLocker locker( mSharedDS ? &mSharedDS->mutex() : nullptr ); - if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mSource->mCrs ) - { - mTransform = QgsCoordinateTransform( mSource->mCrs, mRequest.destinationCrs(), mRequest.transformContext() ); - } + mTransform = mRequest.calculateTransform( mSource->mCrs ); try { mFilterRect = filterRectToSourceCrs( mTransform ); diff --git a/src/core/providers/sensorthings/qgssensorthingsfeatureiterator.cpp b/src/core/providers/sensorthings/qgssensorthingsfeatureiterator.cpp index bb2228e612a3..8b3a9ef0b958 100644 --- a/src/core/providers/sensorthings/qgssensorthingsfeatureiterator.cpp +++ b/src/core/providers/sensorthings/qgssensorthingsfeatureiterator.cpp @@ -48,10 +48,7 @@ QgsSensorThingsFeatureIterator::QgsSensorThingsFeatureIterator( QgsSensorThingsF : QgsAbstractFeatureIteratorFromSource( source, ownSource, request ) , mInterruptionChecker( request.feedback() ) { - if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mSource->sharedData()->crs() ) - { - mTransform = QgsCoordinateTransform( mSource->sharedData()->crs(), mRequest.destinationCrs(), mRequest.transformContext() ); - } + mTransform = mRequest.calculateTransform( mSource->sharedData()->crs() ); try { mFilterRect = filterRectToSourceCrs( mTransform ); diff --git a/src/core/qgscachedfeatureiterator.cpp b/src/core/qgscachedfeatureiterator.cpp index 7436320e5949..4e1ccbb706a4 100644 --- a/src/core/qgscachedfeatureiterator.cpp +++ b/src/core/qgscachedfeatureiterator.cpp @@ -23,10 +23,7 @@ QgsCachedFeatureIterator::QgsCachedFeatureIterator( QgsVectorLayerCache *vlCache : QgsAbstractFeatureIterator( featureRequest ) , mVectorLayerCache( vlCache ) { - if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mVectorLayerCache->sourceCrs() ) - { - mTransform = QgsCoordinateTransform( mVectorLayerCache->sourceCrs(), mRequest.destinationCrs(), mRequest.transformContext() ); - } + mTransform = mRequest.calculateTransform( mVectorLayerCache->sourceCrs() ); try { mFilterRect = filterRectToSourceCrs( mTransform ); @@ -143,10 +140,7 @@ QgsCachedFeatureWriterIterator::QgsCachedFeatureWriterIterator( QgsVectorLayerCa : QgsAbstractFeatureIterator( featureRequest ) , mVectorLayerCache( vlCache ) { - if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mVectorLayerCache->sourceCrs() ) - { - mTransform = QgsCoordinateTransform( mVectorLayerCache->sourceCrs(), mRequest.destinationCrs(), mRequest.transformContext() ); - } + mTransform = mRequest.calculateTransform( mVectorLayerCache->sourceCrs() ); try { mFilterRect = filterRectToSourceCrs( mTransform ); diff --git a/src/core/qgsfeaturesource.cpp b/src/core/qgsfeaturesource.cpp index ee921638e1a9..c848c2e49057 100644 --- a/src/core/qgsfeaturesource.cpp +++ b/src/core/qgsfeaturesource.cpp @@ -137,7 +137,8 @@ QgsFeatureIds QgsFeatureSource::allFeatureIds() const QgsVectorLayer *QgsFeatureSource::materialize( const QgsFeatureRequest &request, QgsFeedback *feedback ) { const Qgis::WkbType outWkbType = ( request.flags() & Qgis::FeatureRequestFlag::NoGeometry ) ? Qgis::WkbType::NoGeometry : wkbType(); - const QgsCoordinateReferenceSystem crs = request.destinationCrs().isValid() ? request.destinationCrs() : sourceCrs(); + const QgsCoordinateReferenceSystem crs = request.coordinateTransform().isValid() ? request.coordinateTransform().destinationCrs() + : request.destinationCrs().isValid() ? request.destinationCrs() : sourceCrs(); const QgsAttributeList requestedAttrs = request.subsetOfAttributes(); diff --git a/src/core/vector/qgsvectorlayerfeatureiterator.cpp b/src/core/vector/qgsvectorlayerfeatureiterator.cpp index 9ab11d40b937..7b01d573377d 100644 --- a/src/core/vector/qgsvectorlayerfeatureiterator.cpp +++ b/src/core/vector/qgsvectorlayerfeatureiterator.cpp @@ -138,11 +138,8 @@ QgsVectorLayerFeatureIterator::QgsVectorLayerFeatureIterator( QgsVectorLayerFeat : QgsAbstractFeatureIteratorFromSource( source, ownSource, request ) , mFetchedFid( false ) { - if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mSource->mCrs ) - { - mTransform = QgsCoordinateTransform( mSource->mCrs, mRequest.destinationCrs(), mRequest.transformContext() ); - mHasValidTransform = mTransform.isValid(); - } + mTransform = mRequest.calculateTransform( mSource->mCrs ); + mHasValidTransform = mTransform.isValid(); // prepare spatial filter geometries for optimal speed // since the mDistanceWithin* constraint member variables are all in the DESTINATION CRS, @@ -238,8 +235,9 @@ QgsVectorLayerFeatureIterator::QgsVectorLayerFeatureIterator( QgsVectorLayerFeat // but we remove any destination CRS parameter - that is handled in QgsVectorLayerFeatureIterator, // not at the provider level. Otherwise virtual fields depending on geometry would have incorrect // values - if ( mRequest.destinationCrs().isValid() ) + if ( mRequest.coordinateTransform().isValid() || mRequest.destinationCrs().isValid() ) { + mProviderRequest.setCoordinateTransform( QgsCoordinateTransform() ); mProviderRequest.setDestinationCrs( QgsCoordinateReferenceSystem(), mRequest.transformContext() ); } diff --git a/src/providers/arcgisrest/qgsafsfeatureiterator.cpp b/src/providers/arcgisrest/qgsafsfeatureiterator.cpp index 1f0e686cea63..ff42c6f616a4 100644 --- a/src/providers/arcgisrest/qgsafsfeatureiterator.cpp +++ b/src/providers/arcgisrest/qgsafsfeatureiterator.cpp @@ -41,10 +41,7 @@ QgsAfsSharedData *QgsAfsFeatureSource::sharedData() const QgsAfsFeatureIterator::QgsAfsFeatureIterator( QgsAfsFeatureSource *source, bool ownSource, const QgsFeatureRequest &request ) : QgsAbstractFeatureIteratorFromSource( source, ownSource, request ) { - if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mSource->sharedData()->crs() ) - { - mTransform = QgsCoordinateTransform( mSource->sharedData()->crs(), mRequest.destinationCrs(), mRequest.transformContext() ); - } + mTransform = mRequest.calculateTransform( mSource->sharedData()->crs() ); try { mFilterRect = filterRectToSourceCrs( mTransform ); diff --git a/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp b/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp index f71bc0dece5f..213b67b4f870 100644 --- a/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp +++ b/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp @@ -41,10 +41,7 @@ QgsDelimitedTextFeatureIterator::QgsDelimitedTextFeatureIterator( QgsDelimitedTe // load it. const bool hasGeometry = mSource->mGeomRep != QgsDelimitedTextProvider::GeomNone; - if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mSource->mCrs ) - { - mTransform = QgsCoordinateTransform( mSource->mCrs, mRequest.destinationCrs(), mRequest.transformContext() ); - } + mTransform = mRequest.calculateTransform( mSource->mCrs ); try { mFilterRect = filterRectToSourceCrs( mTransform ); diff --git a/src/providers/gpx/qgsgpxfeatureiterator.cpp b/src/providers/gpx/qgsgpxfeatureiterator.cpp index 8bd655955a4a..e25af07cf585 100644 --- a/src/providers/gpx/qgsgpxfeatureiterator.cpp +++ b/src/providers/gpx/qgsgpxfeatureiterator.cpp @@ -35,10 +35,7 @@ QgsGPXFeatureIterator::QgsGPXFeatureIterator( QgsGPXFeatureSource *source, bool return; } - if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mSource->mCrs ) - { - mTransform = QgsCoordinateTransform( mSource->mCrs, mRequest.destinationCrs(), mRequest.transformContext() ); - } + mTransform = mRequest.calculateTransform( mSource->mCrs ); try { mFilterRect = filterRectToSourceCrs( mTransform ); diff --git a/src/providers/hana/qgshanafeatureiterator.cpp b/src/providers/hana/qgshanafeatureiterator.cpp index 8a9905994482..8b4b37118598 100644 --- a/src/providers/hana/qgshanafeatureiterator.cpp +++ b/src/providers/hana/qgshanafeatureiterator.cpp @@ -85,8 +85,7 @@ QgsHanaFeatureIterator::QgsHanaFeatureIterator( return; } - if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mSource->mCrs ) - mTransform = QgsCoordinateTransform( mSource->mCrs, mRequest.destinationCrs(), mRequest.transformContext() ); + mTransform = mRequest.calculateTransform( mSource->mCrs ); try { diff --git a/src/providers/mssql/qgsmssqlfeatureiterator.cpp b/src/providers/mssql/qgsmssqlfeatureiterator.cpp index 5a234ec45d1d..71d3fe472147 100644 --- a/src/providers/mssql/qgsmssqlfeatureiterator.cpp +++ b/src/providers/mssql/qgsmssqlfeatureiterator.cpp @@ -39,10 +39,7 @@ QgsMssqlFeatureIterator::QgsMssqlFeatureIterator( QgsMssqlFeatureSource *source, mParser.mIsGeography = mSource->mIsGeography; - if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mSource->mCrs ) - { - mTransform = QgsCoordinateTransform( mSource->mCrs, mRequest.destinationCrs(), mRequest.transformContext() ); - } + mTransform = mRequest.calculateTransform( mSource->mCrs ); try { mFilterRect = filterRectToSourceCrs( mTransform ); diff --git a/src/providers/oracle/qgsoraclefeatureiterator.cpp b/src/providers/oracle/qgsoraclefeatureiterator.cpp index ad3fccb8d0e1..2dcb68c53f84 100644 --- a/src/providers/oracle/qgsoraclefeatureiterator.cpp +++ b/src/providers/oracle/qgsoraclefeatureiterator.cpp @@ -48,10 +48,7 @@ QgsOracleFeatureIterator::QgsOracleFeatureIterator( QgsOracleFeatureSource *sour return; } - if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mSource->mCrs ) - { - mTransform = QgsCoordinateTransform( mSource->mCrs, mRequest.destinationCrs(), mRequest.transformContext() ); - } + mTransform = mRequest.calculateTransform( mSource->mCrs ); try { mFilterRect = filterRectToSourceCrs( mTransform ); diff --git a/src/providers/postgres/qgspostgresfeatureiterator.cpp b/src/providers/postgres/qgspostgresfeatureiterator.cpp index 078ccdc7d2fe..857c630f37f5 100644 --- a/src/providers/postgres/qgspostgresfeatureiterator.cpp +++ b/src/providers/postgres/qgspostgresfeatureiterator.cpp @@ -57,10 +57,7 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresFeatureSource return; } - if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mSource->mCrs ) - { - mTransform = QgsCoordinateTransform( mSource->mCrs, mRequest.destinationCrs(), mRequest.transformContext() ); - } + mTransform = mRequest.calculateTransform( mSource->mCrs ); try { mFilterRect = filterRectToSourceCrs( mTransform ); diff --git a/src/providers/spatialite/qgsspatialitefeatureiterator.cpp b/src/providers/spatialite/qgsspatialitefeatureiterator.cpp index 2fb535ce998d..b9de4bdf8351 100644 --- a/src/providers/spatialite/qgsspatialitefeatureiterator.cpp +++ b/src/providers/spatialite/qgsspatialitefeatureiterator.cpp @@ -51,10 +51,7 @@ QgsSpatiaLiteFeatureIterator::QgsSpatiaLiteFeatureIterator( QgsSpatiaLiteFeature QString fallbackWhereClause; QString whereClause; - if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mSource->mCrs ) - { - mTransform = QgsCoordinateTransform( mSource->mCrs, mRequest.destinationCrs(), mRequest.transformContext() ); - } + mTransform = mRequest.calculateTransform( mSource->mCrs ); try { mFilterRect = filterRectToSourceCrs( mTransform ); diff --git a/src/providers/virtual/qgsvirtuallayerfeatureiterator.cpp b/src/providers/virtual/qgsvirtuallayerfeatureiterator.cpp index 995a2b0e6858..baea2932118e 100644 --- a/src/providers/virtual/qgsvirtuallayerfeatureiterator.cpp +++ b/src/providers/virtual/qgsvirtuallayerfeatureiterator.cpp @@ -43,10 +43,7 @@ QgsVirtualLayerFeatureIterator::QgsVirtualLayerFeatureIterator( QgsVirtualLayerF return; } - if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mSource->mCrs ) - { - mTransform = QgsCoordinateTransform( mSource->mCrs, mRequest.destinationCrs(), mRequest.transformContext() ); - } + mTransform = mRequest.calculateTransform( mSource->mCrs ); try { mFilterRect = filterRectToSourceCrs( mTransform ); diff --git a/src/providers/wfs/qgsbackgroundcachedfeatureiterator.cpp b/src/providers/wfs/qgsbackgroundcachedfeatureiterator.cpp index 51da1b67c059..9047443ca1bf 100644 --- a/src/providers/wfs/qgsbackgroundcachedfeatureiterator.cpp +++ b/src/providers/wfs/qgsbackgroundcachedfeatureiterator.cpp @@ -294,10 +294,7 @@ QgsBackgroundCachedFeatureIterator::QgsBackgroundCachedFeatureIterator( } } - if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mShared->sourceCrs() ) - { - mTransform = QgsCoordinateTransform( mShared->sourceCrs(), mRequest.destinationCrs(), mRequest.transformContext() ); - } + mTransform = mRequest.calculateTransform( mShared->sourceCrs() ); try { mFilterRect = filterRectToSourceCrs( mTransform ); diff --git a/tests/src/python/featuresourcetestbase.py b/tests/src/python/featuresourcetestbase.py index d62f2ba899fb..0bd2fcb422cf 100644 --- a/tests/src/python/featuresourcetestbase.py +++ b/tests/src/python/featuresourcetestbase.py @@ -15,6 +15,7 @@ from qgis.core import ( NULL, QgsCoordinateReferenceSystem, + QgsCoordinateTransform, QgsFeature, QgsFeatureRequest, QgsGeometry, @@ -681,6 +682,17 @@ def testGetFeaturesDistanceWithinTests(self): self.assertEqual(set(features), {2, 5}) self.assertTrue(all_valid) + # using coordinate transform + request = QgsFeatureRequest().setCoordinateTransform( + QgsCoordinateTransform( + self.source.sourceCrs(), + QgsCoordinateReferenceSystem('EPSG:3857'), QgsProject.instance().transformContext() + )).setDistanceWithin(QgsGeometry.fromWkt('LineString (-7035391 11036245, -7622045 11023301, -7763421 15092839)'), 250000) + features = [f['pk'] for f in self.source.getFeatures(request)] + all_valid = (all(f.isValid() for f in self.source.getFeatures(request))) + self.assertEqual(set(features), {2, 5}) + self.assertTrue(all_valid) + # point geometry request = QgsFeatureRequest().setDistanceWithin( QgsGeometry.fromWkt('Point (-68.1 78.1)'), 3.6) @@ -787,7 +799,7 @@ def testRectAndFids(self): self.assertEqual(request.acceptFeature(f), f['pk'] in expected) def testGetFeaturesDestinationCrs(self): - request = QgsFeatureRequest().setDestinationCrs(QgsCoordinateReferenceSystem('epsg:3785'), + request = QgsFeatureRequest().setDestinationCrs(QgsCoordinateReferenceSystem('epsg:3857'), QgsProject.instance().transformContext()) features = {f['pk']: f for f in self.source.getFeatures(request)} # test that features have been reprojected @@ -803,7 +815,7 @@ def testGetFeaturesDestinationCrs(self): # when destination crs is set, filter rect should be in destination crs rect = QgsRectangle(-7650000, 10500000, -7200000, 15000000) - request = QgsFeatureRequest().setDestinationCrs(QgsCoordinateReferenceSystem('epsg:3785'), + request = QgsFeatureRequest().setDestinationCrs(QgsCoordinateReferenceSystem('epsg:3857'), QgsProject.instance().transformContext()).setFilterRect(rect) features = {f['pk']: f for f in self.source.getFeatures(request)} self.assertEqual(set(features.keys()), {2, 4}) @@ -820,6 +832,52 @@ def testGetFeaturesDestinationCrs(self): features = [f for f in self.source.getFeatures(request)] self.assertFalse(features) + def testGetFeaturesCoordinateTransform(self): + request = QgsFeatureRequest().setCoordinateTransform( + QgsCoordinateTransform( + self.source.sourceCrs(), + QgsCoordinateReferenceSystem('epsg:3857'), + QgsProject.instance().transformContext()) + ) + features = {f['pk']: f for f in self.source.getFeatures(request)} + # test that features have been reprojected + self.assertAlmostEqual(features[1].geometry().constGet().x(), -7829322, -5) + self.assertAlmostEqual(features[1].geometry().constGet().y(), 9967753, -5) + self.assertAlmostEqual(features[2].geometry().constGet().x(), -7591989, -5) + self.assertAlmostEqual(features[2].geometry().constGet().y(), 11334232, -5) + self.assertFalse(features[3].hasGeometry()) + self.assertAlmostEqual(features[4].geometry().constGet().x(), -7271389, -5) + self.assertAlmostEqual(features[4].geometry().constGet().y(), 14531322, -5) + self.assertAlmostEqual(features[5].geometry().constGet().x(), -7917376, -5) + self.assertAlmostEqual(features[5].geometry().constGet().y(), 14493008, -5) + + # when destination crs is set, filter rect should be in destination crs + rect = QgsRectangle(-7650000, 10500000, -7200000, 15000000) + request = QgsFeatureRequest().setCoordinateTransform( + QgsCoordinateTransform( + self.source.sourceCrs(), + QgsCoordinateReferenceSystem('epsg:3857'), + QgsProject.instance().transformContext()) + ).setFilterRect(rect) + features = {f['pk']: f for f in self.source.getFeatures(request)} + self.assertEqual(set(features.keys()), {2, 4}) + # test that features have been reprojected + self.assertAlmostEqual(features[2].geometry().constGet().x(), -7591989, -5) + self.assertAlmostEqual(features[2].geometry().constGet().y(), 11334232, -5) + self.assertAlmostEqual(features[4].geometry().constGet().x(), -7271389, -5) + self.assertAlmostEqual(features[4].geometry().constGet().y(), 14531322, -5) + + # bad rect for transform + rect = QgsRectangle(-99999999999, 99999999999, -99999999998, 99999999998) + request = QgsFeatureRequest().setCoordinateTransform( + QgsCoordinateTransform( + self.source.sourceCrs(), + QgsCoordinateReferenceSystem('epsg:28356'), + QgsProject.instance().transformContext()) + ).setFilterRect(rect) + features = [f for f in self.source.getFeatures(request)] + self.assertFalse(features) + def testGetFeaturesLimit(self): it = self.source.getFeatures(QgsFeatureRequest().setLimit(2)) features = [f['pk'] for f in it] From 205d24bb88ab0a48c198446c7a499ab9621fad14 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Fri, 12 Jul 2024 13:30:42 +1000 Subject: [PATCH 5/9] Add method to determine whether a QgsCoordinateTransform has a vertical component --- .../proj/qgscoordinatetransform.sip.in | 8 ++++ .../proj/qgscoordinatetransform.sip.in | 8 ++++ src/core/proj/qgscoordinatetransform.cpp | 5 ++ src/core/proj/qgscoordinatetransform.h | 8 ++++ src/core/proj/qgscoordinatetransform_p.cpp | 3 ++ src/core/proj/qgscoordinatetransform_p.h | 3 ++ .../src/python/test_qgscoordinatetransform.py | 47 +++++++++++++++++++ 7 files changed, 82 insertions(+) diff --git a/python/PyQt6/core/auto_generated/proj/qgscoordinatetransform.sip.in b/python/PyQt6/core/auto_generated/proj/qgscoordinatetransform.sip.in index a0a95c91ff25..dd14aa6c81c0 100644 --- a/python/PyQt6/core/auto_generated/proj/qgscoordinatetransform.sip.in +++ b/python/PyQt6/core/auto_generated/proj/qgscoordinatetransform.sip.in @@ -349,6 +349,14 @@ otherwise points are transformed from destination to source CRS. bool isShortCircuited() const; %Docstring Returns ``True`` if the transform short circuits because the source and destination are equivalent. +%End + + bool hasVerticalComponent() const; +%Docstring +Returns ``True`` if the transform includes a vertical component, i.e. if both the :py:func:`~QgsCoordinateTransform.sourceCrs` +and :py:func:`~QgsCoordinateTransform.destinationCrs` have a vertical axis. + +.. versionadded:: 3.40 %End QString coordinateOperation() const; diff --git a/python/core/auto_generated/proj/qgscoordinatetransform.sip.in b/python/core/auto_generated/proj/qgscoordinatetransform.sip.in index a0a95c91ff25..dd14aa6c81c0 100644 --- a/python/core/auto_generated/proj/qgscoordinatetransform.sip.in +++ b/python/core/auto_generated/proj/qgscoordinatetransform.sip.in @@ -349,6 +349,14 @@ otherwise points are transformed from destination to source CRS. bool isShortCircuited() const; %Docstring Returns ``True`` if the transform short circuits because the source and destination are equivalent. +%End + + bool hasVerticalComponent() const; +%Docstring +Returns ``True`` if the transform includes a vertical component, i.e. if both the :py:func:`~QgsCoordinateTransform.sourceCrs` +and :py:func:`~QgsCoordinateTransform.destinationCrs` have a vertical axis. + +.. versionadded:: 3.40 %End QString coordinateOperation() const; diff --git a/src/core/proj/qgscoordinatetransform.cpp b/src/core/proj/qgscoordinatetransform.cpp index caab2b3623c0..b079d4208d46 100644 --- a/src/core/proj/qgscoordinatetransform.cpp +++ b/src/core/proj/qgscoordinatetransform.cpp @@ -935,6 +935,11 @@ bool QgsCoordinateTransform::isShortCircuited() const return !d->mIsValid || d->mShortCircuit; } +bool QgsCoordinateTransform::hasVerticalComponent() const +{ + return d->mIsValid && d->mHasVerticalComponent; +} + QString QgsCoordinateTransform::coordinateOperation() const { return d->mProjCoordinateOperation; diff --git a/src/core/proj/qgscoordinatetransform.h b/src/core/proj/qgscoordinatetransform.h index 2048d37dfb47..2f45e4af4037 100644 --- a/src/core/proj/qgscoordinatetransform.h +++ b/src/core/proj/qgscoordinatetransform.h @@ -381,6 +381,14 @@ class CORE_EXPORT QgsCoordinateTransform */ bool isShortCircuited() const; + /** + * Returns TRUE if the transform includes a vertical component, i.e. if both the sourceCrs() + * and destinationCrs() have a vertical axis. + * + * \since QGIS 3.40 + */ + bool hasVerticalComponent() const; + /** * Returns a Proj string representing the coordinate operation which will be used to transform * coordinates. diff --git a/src/core/proj/qgscoordinatetransform_p.cpp b/src/core/proj/qgscoordinatetransform_p.cpp index 63e438a90f9e..fc53fd08986f 100644 --- a/src/core/proj/qgscoordinatetransform_p.cpp +++ b/src/core/proj/qgscoordinatetransform_p.cpp @@ -84,6 +84,7 @@ QgsCoordinateTransformPrivate::QgsCoordinateTransformPrivate( const QgsCoordinat , mIsValid( other.mIsValid ) , mShortCircuit( other.mShortCircuit ) , mGeographicToWebMercator( other.mGeographicToWebMercator ) + , mHasVerticalComponent( other.mHasVerticalComponent ) , mSourceCRS( other.mSourceCRS ) , mDestCRS( other.mDestCRS ) , mSourceDatumTransform( other.mSourceDatumTransform ) @@ -163,6 +164,8 @@ bool QgsCoordinateTransformPrivate::initialize() mSourceCRS.isGeographic() && mDestCRS.authid() == QLatin1String( "EPSG:3857" ); + mHasVerticalComponent = mSourceCRS.hasVerticalAxis() && mDestCRS.hasVerticalAxis(); + mSourceIsDynamic = mSourceCRS.isDynamic(); mSourceCoordinateEpoch = mSourceCRS.coordinateEpoch(); mDestIsDynamic = mDestCRS.isDynamic(); diff --git a/src/core/proj/qgscoordinatetransform_p.h b/src/core/proj/qgscoordinatetransform_p.h index 34246fd1b78f..30a3b45aea7f 100644 --- a/src/core/proj/qgscoordinatetransform_p.h +++ b/src/core/proj/qgscoordinatetransform_p.h @@ -91,6 +91,9 @@ class QgsCoordinateTransformPrivate : public QSharedData //! Flag to indicate EPSG:4326 to EPSG:3857 reprojection bool mGeographicToWebMercator = false; + //! Flag to indicate whether the transform has a vertical component + bool mHasVerticalComponent = false; + //! QgsCoordinateReferenceSystem of the source (layer) coordinate system QgsCoordinateReferenceSystem mSourceCRS; diff --git a/tests/src/python/test_qgscoordinatetransform.py b/tests/src/python/test_qgscoordinatetransform.py index bd8a1142adb4..1e9fb914342a 100644 --- a/tests/src/python/test_qgscoordinatetransform.py +++ b/tests/src/python/test_qgscoordinatetransform.py @@ -161,6 +161,53 @@ def testTransformBoundingBoxFullWorldToWebMercator(self): self.assertAlmostEqual(transformedExtent.xMaximum(), 20037508.343, delta=1e-3) self.assertAlmostEqual(transformedExtent.yMaximum(), 44927335.427, delta=1e-3) + def test_has_vertical_component(self): + transform = QgsCoordinateTransform() + self.assertFalse(transform.hasVerticalComponent()) + + # 2d to 2d + transform = QgsCoordinateTransform( + QgsCoordinateReferenceSystem('EPSG:4326'), + QgsCoordinateReferenceSystem('EPSG:3857'), + QgsCoordinateTransformContext() + ) + self.assertFalse(transform.hasVerticalComponent()) + + # 2d to 3d + transform = QgsCoordinateTransform( + QgsCoordinateReferenceSystem('EPSG:4326'), + QgsCoordinateReferenceSystem('EPSG:7843'), + QgsCoordinateTransformContext() + ) + self.assertFalse(transform.hasVerticalComponent()) + + # 3d to 2d + transform = QgsCoordinateTransform( + QgsCoordinateReferenceSystem('EPSG:7843'), + QgsCoordinateReferenceSystem('EPSG:4326'), + QgsCoordinateTransformContext() + ) + self.assertFalse(transform.hasVerticalComponent()) + + # 3d to 3d + transform = QgsCoordinateTransform( + QgsCoordinateReferenceSystem('EPSG:7843'), + QgsCoordinateReferenceSystem.createCompoundCrs( + QgsCoordinateReferenceSystem('EPSG:7844'), + QgsCoordinateReferenceSystem('EPSG:9458'))[0], + QgsCoordinateTransformContext() + ) + self.assertTrue(transform.hasVerticalComponent()) + + transform = QgsCoordinateTransform( + QgsCoordinateReferenceSystem.createCompoundCrs( + QgsCoordinateReferenceSystem('EPSG:7844'), + QgsCoordinateReferenceSystem('EPSG:5711'))[0], + QgsCoordinateReferenceSystem('EPSG:7843'), + QgsCoordinateTransformContext() + ) + self.assertTrue(transform.hasVerticalComponent()) + if __name__ == '__main__': unittest.main() From 594179a14b8fa3afdb399aa8e6e8ce95728fa050 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Fri, 12 Jul 2024 13:40:46 +1000 Subject: [PATCH 6/9] Ensure z values are transformed in iterators when appropriate --- src/core/qgsfeatureiterator.cpp | 2 +- tests/src/python/test_qgsfeatureiterator.py | 194 ++++++++++++++++++++ 2 files changed, 195 insertions(+), 1 deletion(-) diff --git a/src/core/qgsfeatureiterator.cpp b/src/core/qgsfeatureiterator.cpp index e931b3140a67..3753b2938a86 100644 --- a/src/core/qgsfeatureiterator.cpp +++ b/src/core/qgsfeatureiterator.cpp @@ -102,7 +102,7 @@ void QgsAbstractFeatureIterator::geometryToDestinationCrs( QgsFeature &feature, try { QgsGeometry g = feature.geometry(); - g.transform( transform ); + g.transform( transform, Qgis::TransformDirection::Forward, transform.hasVerticalComponent() ); feature.setGeometry( g ); } catch ( QgsCsException & ) diff --git a/tests/src/python/test_qgsfeatureiterator.py b/tests/src/python/test_qgsfeatureiterator.py index 6c81906e5003..a961b90c4131 100644 --- a/tests/src/python/test_qgsfeatureiterator.py +++ b/tests/src/python/test_qgsfeatureiterator.py @@ -23,6 +23,10 @@ QgsPropertyDefinition, QgsVectorLayer, QgsVectorLayerJoinInfo, + QgsPoint, + QgsCoordinateReferenceSystem, + QgsCoordinateTransform, + QgsCoordinateTransformContext, ) import unittest from qgis.testing import start_app, QgisTestCase @@ -533,6 +537,196 @@ def callback(feature): self.assertEqual(res, ['a', 'b']) layer.rollBack() + def test_vertical_transformation_gda2020_to_AVWS(self): + """ + Test vertical transformations are correctly handled during iteration + + GDA2020 to AVWS + """ + + # GDA2020 vertical CRS + vl = QgsVectorLayer('PointZ?crs=EPSG:7843', 'gda2020points', 'memory') + self.assertTrue(vl.isValid()) + self.assertEqual(vl.crs().authid(), 'EPSG:7843') + + self.assertEqual(vl.crs3D().horizontalCrs().authid(), 'EPSG:7843') + + f = QgsFeature() + f.setGeometry(QgsPoint(134.445567853, + -23.445567853, + 5543.325)) + self.assertTrue(vl.dataProvider().addFeature(f)) + + # AVWS + dest_crs, msg = QgsCoordinateReferenceSystem.createCompoundCrs( + QgsCoordinateReferenceSystem('EPSG:7844'), + QgsCoordinateReferenceSystem('EPSG:9458')) + self.assertFalse(msg) + self.assertTrue(dest_crs.isValid()) + self.assertEqual(dest_crs.horizontalCrs().authid(), 'EPSG:7844') + self.assertEqual(dest_crs.verticalCrs().authid(), 'EPSG:9458') + + transform = QgsCoordinateTransform( + vl.crs3D(), + dest_crs, + QgsCoordinateTransformContext() + ) + # for debugging + # self.assertEqual(transform.instantiatedCoordinateOperationDetails().proj, '+proj=pipeline +step +proj=unitconvert +xy_in=deg +xy_out=rad +step +inv +proj=vgridshift +grids=au_ga_AGQG_20201120.tif +multiplier=1 +step +proj=unitconvert +xy_in=rad +xy_out=deg') + + request = QgsFeatureRequest().setCoordinateTransform( + transform) + + transformed_features = list(vl.getFeatures(request)) + self.assertEqual(len(transformed_features), 1) + geom = transformed_features[0].geometry() + self.assertAlmostEqual(geom.constGet().x(), 134.445567853, 6) + self.assertAlmostEqual(geom.constGet().y(), -23.445567853, 6) + # comparing against results from https://geodesyapps.ga.gov.au/avws + self.assertAlmostEqual(geom.constGet().z(), 5524.13969, 3) + + def test_vertical_transformation_AVWS_to_gda2020(self): + """ + Test vertical transformations are correctly handled during iteration + + AVWS to GDA2020 + """ + + # GDA2020 vertical CRS + vl = QgsVectorLayer('PointZ?crs=EPSG:7844', 'gda2020points', 'memory') + self.assertTrue(vl.isValid()) + + f = QgsFeature() + f.setGeometry(QgsPoint(134.445567853, + -23.445567853, + 5524.13969)) + self.assertTrue(vl.dataProvider().addFeature(f)) + + # AVWS + source_crs, msg = QgsCoordinateReferenceSystem.createCompoundCrs( + QgsCoordinateReferenceSystem('EPSG:7844'), + QgsCoordinateReferenceSystem('EPSG:9458')) + self.assertFalse(msg) + self.assertTrue(source_crs.isValid()) + self.assertEqual(source_crs.horizontalCrs().authid(), 'EPSG:7844') + self.assertEqual(source_crs.verticalCrs().authid(), 'EPSG:9458') + + # dest CRS is GDA2020 + + transform = QgsCoordinateTransform( + source_crs, + QgsCoordinateReferenceSystem('EPSG:7843'), + QgsCoordinateTransformContext() + ) + # for debugging + # self.assertEqual(transform.instantiatedCoordinateOperationDetails().proj, '+proj=pipeline +step +proj=unitconvert +xy_in=deg +xy_out=rad +step +inv +proj=vgridshift +grids=au_ga_AGQG_20201120.tif +multiplier=1 +step +proj=unitconvert +xy_in=rad +xy_out=deg') + + request = QgsFeatureRequest().setCoordinateTransform( + transform) + + transformed_features = list(vl.getFeatures(request)) + self.assertEqual(len(transformed_features), 1) + geom = transformed_features[0].geometry() + self.assertAlmostEqual(geom.constGet().x(), 134.445567853, 6) + self.assertAlmostEqual(geom.constGet().y(), -23.445567853, 6) + # comparing against results from https://geodesyapps.ga.gov.au/avws + self.assertAlmostEqual(geom.constGet().z(), 5543.325, 3) + + def test_vertical_transformation_gda2020_to_AHD(self): + """ + Test vertical transformations are correctly handled during iteration + + GDA2020 to AHD + """ + + # GDA2020 vertical CRS + vl = QgsVectorLayer('PointZ?crs=EPSG:7843', 'gda2020points', 'memory') + self.assertTrue(vl.isValid()) + self.assertEqual(vl.crs().authid(), 'EPSG:7843') + + self.assertEqual(vl.crs3D().horizontalCrs().authid(), 'EPSG:7843') + + f = QgsFeature() + f.setGeometry(QgsPoint(134.445567853, + -23.445567853, + 5543.325)) + self.assertTrue(vl.dataProvider().addFeature(f)) + + # AHD + dest_crs, msg = QgsCoordinateReferenceSystem.createCompoundCrs( + QgsCoordinateReferenceSystem('EPSG:7844'), + QgsCoordinateReferenceSystem('EPSG:5711')) + self.assertFalse(msg) + self.assertTrue(dest_crs.isValid()) + self.assertEqual(dest_crs.horizontalCrs().authid(), 'EPSG:7844') + self.assertEqual(dest_crs.verticalCrs().authid(), 'EPSG:5711') + + transform = QgsCoordinateTransform( + vl.crs3D(), + dest_crs, + QgsCoordinateTransformContext() + ) + # for debugging + # self.assertEqual(transform.instantiatedCoordinateOperationDetails().proj, '+proj=pipeline +step +proj=unitconvert +xy_in=deg +xy_out=rad +step +inv +proj=vgridshift +grids=au_ga_AGQG_20201120.tif +multiplier=1 +step +proj=unitconvert +xy_in=rad +xy_out=deg') + + request = QgsFeatureRequest().setCoordinateTransform( + transform) + + transformed_features = list(vl.getFeatures(request)) + self.assertEqual(len(transformed_features), 1) + geom = transformed_features[0].geometry() + self.assertAlmostEqual(geom.constGet().x(), 134.445567853, 6) + self.assertAlmostEqual(geom.constGet().y(), -23.445567853, 6) + # comparing against results from https://geodesyapps.ga.gov.au/ausgeoid2020 + self.assertAlmostEqual(geom.constGet().z(), 5523.598, 3) + + def test_vertical_transformation_AHD_to_gda2020(self): + """ + Test vertical transformations are correctly handled during iteration + + AHD to GDA2020 + """ + + # GDA2020 vertical CRS + vl = QgsVectorLayer('PointZ?crs=EPSG:7844', 'gda2020points', 'memory') + self.assertTrue(vl.isValid()) + + f = QgsFeature() + f.setGeometry(QgsPoint(134.445567853, + -23.445567853, + 5523.598)) + self.assertTrue(vl.dataProvider().addFeature(f)) + + # AHD + source_crs, msg = QgsCoordinateReferenceSystem.createCompoundCrs( + QgsCoordinateReferenceSystem('EPSG:7844'), + QgsCoordinateReferenceSystem('EPSG:5711')) + self.assertFalse(msg) + self.assertTrue(source_crs.isValid()) + self.assertEqual(source_crs.horizontalCrs().authid(), 'EPSG:7844') + self.assertEqual(source_crs.verticalCrs().authid(), 'EPSG:5711') + + # dest CRS is GDA2020 + + transform = QgsCoordinateTransform( + source_crs, + QgsCoordinateReferenceSystem('EPSG:7843'), + QgsCoordinateTransformContext() + ) + # for debugging + # self.assertEqual(transform.instantiatedCoordinateOperationDetails().proj, '+proj=pipeline +step +proj=unitconvert +xy_in=deg +xy_out=rad +step +inv +proj=vgridshift +grids=au_ga_AGQG_20201120.tif +multiplier=1 +step +proj=unitconvert +xy_in=rad +xy_out=deg') + + request = QgsFeatureRequest().setCoordinateTransform( + transform) + + transformed_features = list(vl.getFeatures(request)) + self.assertEqual(len(transformed_features), 1) + geom = transformed_features[0].geometry() + self.assertAlmostEqual(geom.constGet().x(), 134.445567853, 6) + self.assertAlmostEqual(geom.constGet().y(), -23.445567853, 6) + # comparing against results from https://geodesyapps.ga.gov.au/avws + self.assertAlmostEqual(geom.constGet().z(), 5543.325, 3) + if __name__ == '__main__': unittest.main() From 3dae6292d76f56b3edc825e76b00c5be84398609 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Sat, 13 Jul 2024 09:03:59 +1000 Subject: [PATCH 7/9] Fix some python tests --- tests/src/python/provider_python.py | 4 +--- tests/src/python/test_provider_gpx.py | 4 ++++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/src/python/provider_python.py b/tests/src/python/provider_python.py index acfc4af96621..2b99e20e1a44 100644 --- a/tests/src/python/provider_python.py +++ b/tests/src/python/provider_python.py @@ -43,9 +43,7 @@ def __init__(self, source, request): self._request = request if request is not None else QgsFeatureRequest() self._source = source self._index = 0 - self._transform = QgsCoordinateTransform() - if self._request.destinationCrs().isValid() and self._request.destinationCrs() != self._source._provider.crs(): - self._transform = QgsCoordinateTransform(self._source._provider.crs(), self._request.destinationCrs(), self._request.transformContext()) + self._transform = request.calculateTransform(self._source._provider.crs()) try: self._filter_rect = self.filterRectToSourceCrs(self._transform) except QgsCsException as e: diff --git a/tests/src/python/test_provider_gpx.py b/tests/src/python/test_provider_gpx.py index 6ae81a077a1b..cad0a3cbc390 100644 --- a/tests/src/python/test_provider_gpx.py +++ b/tests/src/python/test_provider_gpx.py @@ -63,6 +63,10 @@ def testGetFeatures(self): def testGetFeaturesDestinationCrs(self): pass + @unittest.skip('Base provider test is not suitable for GPX provider') + def testGetFeaturesCoordinateTransform(self): + pass + @unittest.skip('Base provider test is not suitable for GPX provider') def testGetFeaturesLimit(self): pass From d6f2b78cf3fa2a43efdb69640a5a344ea592504c Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Sat, 13 Jul 2024 09:43:13 +1000 Subject: [PATCH 8/9] Skip tests when grid not available --- tests/src/python/test_qgsfeatureiterator.py | 28 +++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/src/python/test_qgsfeatureiterator.py b/tests/src/python/test_qgsfeatureiterator.py index a961b90c4131..9938628b5012 100644 --- a/tests/src/python/test_qgsfeatureiterator.py +++ b/tests/src/python/test_qgsfeatureiterator.py @@ -27,6 +27,7 @@ QgsCoordinateReferenceSystem, QgsCoordinateTransform, QgsCoordinateTransformContext, + QgsDatumTransform, ) import unittest from qgis.testing import start_app, QgisTestCase @@ -566,6 +567,12 @@ def test_vertical_transformation_gda2020_to_AVWS(self): self.assertEqual(dest_crs.horizontalCrs().authid(), 'EPSG:7844') self.assertEqual(dest_crs.verticalCrs().authid(), 'EPSG:9458') + available_operations = QgsDatumTransform.operations(vl.crs3D(), dest_crs) + self.assertEqual(len(available_operations[0].grids), 1) + self.assertEqual(available_operations[0].grids[0].shortName, 'au_ga_AGQG_20201120.tif') + if not available_operations[0].isAvailable: + self.skipTest(f'Required grid {available_operations[0].grids[0].shortName} not available on system') + transform = QgsCoordinateTransform( vl.crs3D(), dest_crs, @@ -611,6 +618,13 @@ def test_vertical_transformation_AVWS_to_gda2020(self): self.assertEqual(source_crs.horizontalCrs().authid(), 'EPSG:7844') self.assertEqual(source_crs.verticalCrs().authid(), 'EPSG:9458') + available_operations = QgsDatumTransform.operations(source_crs, + QgsCoordinateReferenceSystem('EPSG:7843')) + self.assertEqual(len(available_operations[0].grids), 1) + self.assertEqual(available_operations[0].grids[0].shortName, 'au_ga_AGQG_20201120.tif') + if not available_operations[0].isAvailable: + self.skipTest(f'Required grid {available_operations[0].grids[0].shortName} not available on system') + # dest CRS is GDA2020 transform = QgsCoordinateTransform( @@ -661,6 +675,13 @@ def test_vertical_transformation_gda2020_to_AHD(self): self.assertEqual(dest_crs.horizontalCrs().authid(), 'EPSG:7844') self.assertEqual(dest_crs.verticalCrs().authid(), 'EPSG:5711') + available_operations = QgsDatumTransform.operations(vl.crs3D(), + dest_crs) + self.assertEqual(len(available_operations[0].grids), 1) + self.assertEqual(available_operations[0].grids[0].shortName, 'au_ga_AUSGeoid2020_20180201.tif') + if not available_operations[0].isAvailable: + self.skipTest(f'Required grid {available_operations[0].grids[0].shortName} not available on system') + transform = QgsCoordinateTransform( vl.crs3D(), dest_crs, @@ -708,6 +729,13 @@ def test_vertical_transformation_AHD_to_gda2020(self): # dest CRS is GDA2020 + available_operations = QgsDatumTransform.operations(source_crs, + QgsCoordinateReferenceSystem('EPSG:7843')) + self.assertEqual(len(available_operations[0].grids), 1) + self.assertEqual(available_operations[0].grids[0].shortName, 'au_ga_AUSGeoid2020_20180201.tif') + if not available_operations[0].isAvailable: + self.skipTest(f'Required grid {available_operations[0].grids[0].shortName} not available on system') + transform = QgsCoordinateTransform( source_crs, QgsCoordinateReferenceSystem('EPSG:7843'), From 87fad8624977cc07fca421b0705c7199bdd5a649 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Sat, 13 Jul 2024 09:50:01 +1000 Subject: [PATCH 9/9] Add vertical transform test which doesn't require grid files --- tests/src/python/test_qgsfeatureiterator.py | 39 +++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/src/python/test_qgsfeatureiterator.py b/tests/src/python/test_qgsfeatureiterator.py index 9938628b5012..488d83fedaf7 100644 --- a/tests/src/python/test_qgsfeatureiterator.py +++ b/tests/src/python/test_qgsfeatureiterator.py @@ -538,6 +538,45 @@ def callback(feature): self.assertEqual(res, ['a', 'b']) layer.rollBack() + def test_vertical_transformation_4978_to_4979(self): + """ + Test vertical transformations are correctly handled during iteration + + EPSG:4978 to EPSG:4979 + """ + + vl = QgsVectorLayer('PointZ?crs=EPSG:4978', 'gda2020points', 'memory') + self.assertTrue(vl.isValid()) + self.assertEqual(vl.crs().authid(), 'EPSG:4978') + + self.assertEqual(vl.crs3D().horizontalCrs().authid(), 'EPSG:4978') + + f = QgsFeature() + f.setGeometry(QgsPoint(134.445567853, + -23.445567853, + 5543.325)) + self.assertTrue(vl.dataProvider().addFeature(f)) + + dest_crs = QgsCoordinateReferenceSystem('EPSG:4979') + self.assertTrue(dest_crs.isValid()) + self.assertEqual(dest_crs.horizontalCrs().authid(), 'EPSG:4979') + + transform = QgsCoordinateTransform( + vl.crs3D(), + dest_crs, + QgsCoordinateTransformContext() + ) + + request = QgsFeatureRequest().setCoordinateTransform( + transform) + + transformed_features = list(vl.getFeatures(request)) + self.assertEqual(len(transformed_features), 1) + geom = transformed_features[0].geometry() + self.assertAlmostEqual(geom.constGet().x(), -9.8921668708, 4) + self.assertAlmostEqual(geom.constGet().y(), 89.839008, 4) + self.assertAlmostEqual(geom.constGet().z(), -6351023.00373, 3) + def test_vertical_transformation_gda2020_to_AVWS(self): """ Test vertical transformations are correctly handled during iteration