Handle request destinationCrs in QgsVectorLayerFeatureIterator

qgis · Jun 8, 2017 · b6e1eea · b6e1eea
1 parent a989235
commit b6e1eea
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 27 deletions.
diff --git a/python/core/qgsvectorlayerfeatureiterator.sip b/python/core/qgsvectorlayerfeatureiterator.sip
@@ -141,6 +141,7 @@ Setup the simplification of geometries to fetch using the specified simplify met
 
 
 
+
   private:
     QgsVectorLayerFeatureIterator( const QgsVectorLayerFeatureIterator &rhs );
 };

diff --git a/src/core/qgsvectorlayerfeatureiterator.cpp b/src/core/qgsvectorlayerfeatureiterator.cpp
@@ -109,6 +109,17 @@ QgsVectorLayerFeatureIterator::QgsVectorLayerFeatureIterator( QgsVectorLayerFeat
   , mFetchedFid( false )
   , mInterruptionChecker( nullptr )
 {
+  if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mSource->mCrs )
+  {
+    mTransform = QgsCoordinateTransform( mSource->mCrs, mRequest.destinationCrs() );
+  }
+  mFilterRect = transformedFilterRect( mTransform );
+  if ( !mFilterRect.isNull() )
+  {
+    // update request to be the unprojected filter rect
+    mRequest.setFilterRect( mFilterRect );
+  }
+
   if ( mRequest.filterType() == QgsFeatureRequest::FilterExpression )
   {
     mRequest.expressionContext()->setFields( mSource->mFields );
@@ -129,6 +140,13 @@ QgsVectorLayerFeatureIterator::QgsVectorLayerFeatureIterator( QgsVectorLayerFeat
 
   // by default provider's request is the same
   mProviderRequest = mRequest;
+  // 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() )
+  {
+    mProviderRequest.setDestinationCrs( QgsCoordinateReferenceSystem() );
+  }
 
   if ( mProviderRequest.flags() & QgsFeatureRequest::SubsetOfAttributes )
   {
@@ -253,7 +271,7 @@ bool QgsVectorLayerFeatureIterator::fetchFeature( QgsFeature &f )
     if ( mFetchedFid )
       return false;
     bool res = nextFeatureFid( f );
-    if ( res && testFeature( f ) )
+    if ( res && postProcessFeature( f ) )
     {
       mFetchedFid = true;
       return res;
@@ -264,7 +282,7 @@ bool QgsVectorLayerFeatureIterator::fetchFeature( QgsFeature &f )
     }
   }
 
-  if ( !mRequest.filterRect().isNull() )
+  if ( !mFilterRect.isNull() )
   {
     if ( fetchNextChangedGeomFeature( f ) )
       return true;
@@ -327,7 +345,7 @@ bool QgsVectorLayerFeatureIterator::fetchFeature( QgsFeature &f )
     if ( !( mRequest.flags() & QgsFeatureRequest::NoGeometry ) )
       updateFeatureGeometry( f );
 
-    if ( !testFeature( f ) )
+    if ( !postProcessFeature( f ) )
       continue;
 
     return true;
@@ -387,15 +405,17 @@ bool QgsVectorLayerFeatureIterator::fetchNextAddedFeature( QgsFeature &f )
       // must have changed geometry outside rectangle
       continue;
 
-    if ( !mRequest.acceptFeature( *mFetchAddedFeaturesIt ) )
+    useAddedFeature( *mFetchAddedFeaturesIt, f );
+
+    // can't test for feature acceptance until after calling useAddedFeature
+    // since acceptFeature may rely on virtual fields
+    if ( !mRequest.acceptFeature( f ) )
       // skip features which are not accepted by the filter
       continue;
 
-    if ( !testFeature( *mFetchAddedFeaturesIt ) )
+    if ( !postProcessFeature( f ) )
       continue;
 
-    useAddedFeature( *mFetchAddedFeaturesIt, f );
-
     return true;
   }
 
@@ -406,23 +426,13 @@ bool QgsVectorLayerFeatureIterator::fetchNextAddedFeature( QgsFeature &f )
 
 void QgsVectorLayerFeatureIterator::useAddedFeature( const QgsFeature &src, QgsFeature &f )
 {
-  f.setId( src.id() );
+  // since QgsFeature is implicitly shared, it's more efficient to just copy the
+  // whole feature, even if flags like NoGeometry or a subset of attributes is set at the request.
+  // This helps potentially avoid an unnecessary detach of the feature
+  f = src;
   f.setValid( true );
   f.setFields( mSource->mFields );
 
-  if ( src.hasGeometry() && !( mRequest.flags() & QgsFeatureRequest::NoGeometry ) )
-  {
-    f.setGeometry( src.geometry() );
-  }
-  else
-  {
-    f.clearGeometry();
-  }
-
-  // TODO[MD]: if subset set just some attributes
-
-  f.setAttributes( src.attributes() );
-
   if ( mHasVirtualAttributes )
     addVirtualAttributes( f );
 }
@@ -442,7 +452,7 @@ bool QgsVectorLayerFeatureIterator::fetchNextChangedGeomFeature( QgsFeature &f )
 
     mFetchConsidered << fid;
 
-    if ( !mRequest.filterRect().isNull() && !mFetchChangedGeomIt->intersects( mRequest.filterRect() ) )
+    if ( !mFilterRect.isNull() && !mFetchChangedGeomIt->intersects( mFilterRect ) )
       // skip changed geometries not in rectangle and don't check again
       continue;
 
@@ -457,7 +467,7 @@ bool QgsVectorLayerFeatureIterator::fetchNextChangedGeomFeature( QgsFeature &f )
       }
     }
 
-    if ( testFeature( f ) )
+    if ( postProcessFeature( f ) )
     {
       // return complete feature
       mFetchChangedGeomIt++;
@@ -484,7 +494,7 @@ bool QgsVectorLayerFeatureIterator::fetchNextChangedAttributeFeature( QgsFeature
       addVirtualAttributes( f );
 
     mRequest.expressionContext()->setFeature( f );
-    if ( mRequest.filterExpression()->evaluate( mRequest.expressionContext() ).toBool() && testFeature( f ) )
+    if ( mRequest.filterExpression()->evaluate( mRequest.expressionContext() ).toBool() && postProcessFeature( f ) )
     {
       return true;
     }
@@ -703,9 +713,11 @@ void QgsVectorLayerFeatureIterator::createOrderedJoinList()
   }
 }
 
-bool QgsVectorLayerFeatureIterator::testFeature( const QgsFeature &feature )
+bool QgsVectorLayerFeatureIterator::postProcessFeature( QgsFeature &feature )
 {
   bool result = checkGeometryValidity( feature );
+  if ( result )
+    transformFeatureGeometry( feature, mTransform );
   return result;
 }
 

diff --git a/src/core/qgsvectorlayerfeatureiterator.h b/src/core/qgsvectorlayerfeatureiterator.h
@@ -208,6 +208,9 @@ class CORE_EXPORT QgsVectorLayerFeatureIterator : public QgsAbstractFeatureItera
     QgsFeatureRequest mChangedFeaturesRequest;
     QgsFeatureIterator mChangedFeaturesIterator;
 
+    QgsRectangle mFilterRect;
+    QgsCoordinateTransform mTransform;
+
     // only related to editing
     QSet<QgsFeatureId> mFetchConsidered;
     QgsGeometryMap::ConstIterator mFetchChangedGeomIt;
@@ -250,9 +253,9 @@ class CORE_EXPORT QgsVectorLayerFeatureIterator : public QgsAbstractFeatureItera
     void createOrderedJoinList();
 
     /**
-     * Performs any feature based validity checking, e.g. checking for geometry validity.
+     * Performs any post-processing (such as transformation) and feature based validity checking, e.g. checking for geometry validity.
      */
-    bool testFeature( const QgsFeature &feature );
+    bool postProcessFeature( QgsFeature &feature );
 
     /**
      * Checks a feature's geometry for validity, if requested in feature request.

diff --git a/tests/src/python/test_qgsvectorlayer.py b/tests/src/python/test_qgsvectorlayer.py
@@ -2364,6 +2364,32 @@ def testQgsVectorLayerSelectedFeatureSource(self):
         ids = set([f.id() for f in source.getFeatures()])
         self.assertEqual(ids, {f1.id(), f3.id(), f5.id()})
 
+    def testFeatureRequestWithReprojectionAndVirtualFields(self):
+        layer = self.getSource()
+        field = QgsField('virtual', QVariant.Double)
+        layer.addExpressionField('$x', field)
+        virtual_values = [f['virtual'] for f in layer.getFeatures()]
+        self.assertAlmostEqual(virtual_values[0], -71.123, 2)
+        self.assertEqual(virtual_values[1], NULL)
+        self.assertAlmostEqual(virtual_values[2], -70.332, 2)
+        self.assertAlmostEqual(virtual_values[3], -68.2, 2)
+        self.assertAlmostEqual(virtual_values[4], -65.32, 2)
+
+        # repeat, with reprojection on request
+        request = QgsFeatureRequest().setDestinationCrs(QgsCoordinateReferenceSystem('epsg:3785'))
+        features = [f for f in layer.getFeatures(request)]
+        # virtual field value should not change, even though geometry has
+        self.assertAlmostEqual(features[0]['virtual'], -71.123, 2)
+        self.assertAlmostEqual(features[0].geometry().geometry().x(), -7917376, -5)
+        self.assertEqual(features[1]['virtual'], NULL)
+        self.assertFalse(features[1].hasGeometry())
+        self.assertAlmostEqual(features[2]['virtual'], -70.332, 2)
+        self.assertAlmostEqual(features[2].geometry().geometry().x(), -7829322, -5)
+        self.assertAlmostEqual(features[3]['virtual'], -68.2, 2)
+        self.assertAlmostEqual(features[3].geometry().geometry().x(), -7591989, -5)
+        self.assertAlmostEqual(features[4]['virtual'], -65.32, 2)
+        self.assertAlmostEqual(features[4].geometry().geometry().x(), -7271389, -5)
+
 
 class TestQgsVectorLayerSourceAddedFeaturesInBuffer(unittest.TestCase, FeatureSourceTestCase):