Implement provider side FilterFids iterators for OGR provider

Makes some operations with OGR sources magnitudes faster, ie zoom to 20 selected features in a 4 million point dataset: before: 14 seconds of blocked gui after: instant Win!
qgis · Aug 4, 2016 · 1f02fd4 · 1f02fd4
1 parent 00a8fea
commit 1f02fd4
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 13 deletions.
diff --git a/src/providers/ogr/qgsogrfeatureiterator.cpp b/src/providers/ogr/qgsogrfeatureiterator.cpp
@@ -43,6 +43,8 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource* source, bool
     , mSubsetStringSet( false )
     , mFetchGeometry( false )
     , mExpressionCompiled( false )
+    , mFilterFids( mRequest.filterFids() )
+    , mFilterFidsIt( mFilterFids.constBegin() )
 {
   mConn = QgsOgrConnPool::instance()->acquireConnection( mSource->mProvider->dataSourceUri() );
   if ( !mConn->ds )
@@ -166,6 +168,22 @@ bool QgsOgrFeatureIterator::nextFeatureFilterExpression( QgsFeature& f )
     return fetchFeature( f );
 }
 
+bool QgsOgrFeatureIterator::fetchFeatureWithId( QgsFeatureId id, QgsFeature& feature ) const
+{
+  feature.setValid( false );
+  OGRFeatureH fet = OGR_L_GetFeature( ogrLayer, FID_TO_NUMBER( id ) );
+  if ( !fet )
+  {
+    return false;
+  }
+
+  if ( readFeature( fet, feature ) )
+    OGR_F_Destroy( fet );
+
+  feature.setValid( true );
+  return true;
+}
+
 bool QgsOgrFeatureIterator::fetchFeature( QgsFeature& feature )
 {
   feature.setValid( false );
@@ -175,19 +193,26 @@ bool QgsOgrFeatureIterator::fetchFeature( QgsFeature& feature )
 
   if ( mRequest.filterType() == QgsFeatureRequest::FilterFid )
   {
-    OGRFeatureH fet = OGR_L_GetFeature( ogrLayer, FID_TO_NUMBER( mRequest.filterFid() ) );
-    if ( !fet )
+    bool result = fetchFeatureWithId( mRequest.filterFid(), feature );
+    close(); // the feature has been read or was not found: we have finished here
+    return result;
+  }
+  else if ( mRequest.filterType() == QgsFeatureRequest::FilterFids )
+  {
+    if ( mFilterFidsIt == mFilterFids.constEnd() )
     {
       close();
       return false;
     }
-
-    if ( readFeature( fet, feature ) )
-      OGR_F_Destroy( fet );
-
-    feature.setValid( true );
-    close(); // the feature has been read: we have finished here
-    return true;
+    else
+    {
+      QgsFeatureId nextId = *mFilterFidsIt;
+      mFilterFidsIt++;
+      bool result = fetchFeatureWithId( nextId, feature );
+      if ( !result )
+        close();
+      return result;
+    }
   }
 
   OGRFeatureH fet;
@@ -220,6 +245,8 @@ bool QgsOgrFeatureIterator::rewind()
 
   OGR_L_ResetReading( ogrLayer );
 
+  mFilterFidsIt = mFilterFids.constBegin();
+
   return true;
 }
 
@@ -246,7 +273,7 @@ bool QgsOgrFeatureIterator::close()
 }
 
 
-void QgsOgrFeatureIterator::getFeatureAttribute( OGRFeatureH ogrFet, QgsFeature & f, int attindex )
+void QgsOgrFeatureIterator::getFeatureAttribute( OGRFeatureH ogrFet, QgsFeature & f, int attindex ) const
 {
   if ( mSource->mFirstFieldIsFid && attindex == 0 )
   {
@@ -264,7 +291,7 @@ void QgsOgrFeatureIterator::getFeatureAttribute( OGRFeatureH ogrFet, QgsFeature
 }
 
 
-bool QgsOgrFeatureIterator::readFeature( OGRFeatureH fet, QgsFeature& feature )
+bool QgsOgrFeatureIterator::readFeature( OGRFeatureH fet, QgsFeature& feature ) const
 {
   feature.setFeatureId( OGR_F_GetFID( fet ) );
   feature.initAttributes( mSource->mFields.count() );

diff --git a/src/providers/ogr/qgsogrfeatureiterator.h b/src/providers/ogr/qgsogrfeatureiterator.h
@@ -69,10 +69,10 @@ class QgsOgrFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsOgr
     //! fetch next feature filter expression
     bool nextFeatureFilterExpression( QgsFeature& f ) override;
 
-    bool readFeature( OGRFeatureH fet, QgsFeature& feature );
+    bool readFeature( OGRFeatureH fet, QgsFeature& feature ) const;
 
     //! Get an attribute associated with a feature
-    void getFeatureAttribute( OGRFeatureH ogrFet, QgsFeature & f, int attindex );
+    void getFeatureAttribute( OGRFeatureH ogrFet, QgsFeature & f, int attindex ) const;
 
     bool mFeatureFetched;
 
@@ -86,6 +86,10 @@ class QgsOgrFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsOgr
 
   private:
     bool mExpressionCompiled;
+    QgsFeatureIds mFilterFids;
+    QgsFeatureIds::const_iterator mFilterFidsIt;
+
+    bool fetchFeatureWithId( QgsFeatureId id, QgsFeature& feature ) const;
 };
 
 #endif // QGSOGRFEATUREITERATOR_H
diff --git a/tests/src/python/providertestbase.py b/tests/src/python/providertestbase.py
@@ -387,6 +387,26 @@ def testGetFeaturesFidsTests(self):
         expected = set([])
         assert result == expected, 'Expected {} and got {} when testing for feature IDs filter'.format(expected, result)
 
+        # Rewind mid-way
+        request = QgsFeatureRequest().setFilterFids([fids[1], fids[3], fids[4]])
+        feature_it = self.provider.getFeatures(request)
+        feature = QgsFeature()
+        feature.setValid(True)
+        self.assertTrue(feature_it.nextFeature(feature))
+        self.assertEqual(feature.id(), fids[1])
+        self.assertTrue(feature.isValid())
+        # rewind
+        self.assertTrue(feature_it.rewind())
+        self.assertTrue(feature_it.nextFeature(feature))
+        self.assertEqual(feature.id(), fids[1])
+        self.assertTrue(feature.isValid())
+        # grab all features
+        self.assertTrue(feature_it.nextFeature(feature))
+        self.assertTrue(feature_it.nextFeature(feature))
+        # none left
+        self.assertFalse(feature_it.nextFeature(feature))
+        self.assertFalse(feature.isValid())
+
     def testGetFeaturesFilterRectTests(self):
         extent = QgsRectangle(-70, 67, -60, 80)
         request = QgsFeatureRequest().setFilterRect(extent)