Skip to content

Commit 9dbd969

Browse files
committedAug 9, 2016
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 (cherry-picked from 1f02fd4)
1 parent 6436a75 commit 9dbd969

File tree

4 files changed

+71
-21
lines changed

4 files changed

+71
-21
lines changed
 

‎src/providers/ogr/qgsogrfeatureiterator.cpp

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource* source, bool
4343
, mFetchGeometry( false )
4444
, mGeometrySimplifier( nullptr )
4545
, mExpressionCompiled( false )
46+
, mFilterFids( mRequest.filterFids() )
47+
, mFilterFidsIt( mFilterFids.constBegin() )
4648
{
4749
mConn = QgsOgrConnPool::instance()->acquireConnection( mSource->mProvider->dataSourceUri() );
4850
if ( !mConn->ds )
@@ -213,6 +215,22 @@ bool QgsOgrFeatureIterator::providerCanSimplify( QgsSimplifyMethod::MethodType m
213215
return false;
214216
}
215217

218+
bool QgsOgrFeatureIterator::fetchFeatureWithId( QgsFeatureId id, QgsFeature& feature ) const
219+
{
220+
feature.setValid( false );
221+
OGRFeatureH fet = OGR_L_GetFeature( ogrLayer, FID_TO_NUMBER( id ) );
222+
if ( !fet )
223+
{
224+
return false;
225+
}
226+
227+
if ( readFeature( fet, feature ) )
228+
OGR_F_Destroy( fet );
229+
230+
feature.setValid( true );
231+
return true;
232+
}
233+
216234
bool QgsOgrFeatureIterator::fetchFeature( QgsFeature& feature )
217235
{
218236
feature.setValid( false );
@@ -222,19 +240,22 @@ bool QgsOgrFeatureIterator::fetchFeature( QgsFeature& feature )
222240

223241
if ( mRequest.filterType() == QgsFeatureRequest::FilterFid )
224242
{
225-
OGRFeatureH fet = OGR_L_GetFeature( ogrLayer, FID_TO_NUMBER( mRequest.filterFid() ) );
226-
if ( !fet )
243+
bool result = fetchFeatureWithId( mRequest.filterFid(), feature );
244+
close(); // the feature has been read or was not found: we have finished here
245+
return result;
246+
}
247+
else if ( mRequest.filterType() == QgsFeatureRequest::FilterFids )
248+
{
249+
while ( mFilterFidsIt != mFilterFids.constEnd() )
227250
{
228-
close();
229-
return false;
230-
}
231-
232-
if ( readFeature( fet, feature ) )
233-
OGR_F_Destroy( fet );
251+
QgsFeatureId nextId = *mFilterFidsIt;
252+
mFilterFidsIt++;
234253

235-
feature.setValid( true );
236-
close(); // the feature has been read: we have finished here
237-
return true;
254+
if ( fetchFeatureWithId( nextId, feature ) )
255+
return true;
256+
}
257+
close();
258+
return false;
238259
}
239260

240261
OGRFeatureH fet;
@@ -267,6 +288,8 @@ bool QgsOgrFeatureIterator::rewind()
267288

268289
OGR_L_ResetReading( ogrLayer );
269290

291+
mFilterFidsIt = mFilterFids.constBegin();
292+
270293
return true;
271294
}
272295

@@ -293,7 +316,7 @@ bool QgsOgrFeatureIterator::close()
293316
}
294317

295318

296-
void QgsOgrFeatureIterator::getFeatureAttribute( OGRFeatureH ogrFet, QgsFeature & f, int attindex )
319+
void QgsOgrFeatureIterator::getFeatureAttribute( OGRFeatureH ogrFet, QgsFeature & f, int attindex ) const
297320
{
298321
OGRFieldDefnH fldDef = OGR_F_GetFieldDefnRef( ogrFet, attindex );
299322

@@ -351,7 +374,7 @@ void QgsOgrFeatureIterator::getFeatureAttribute( OGRFeatureH ogrFet, QgsFeature
351374
}
352375

353376

354-
bool QgsOgrFeatureIterator::readFeature( OGRFeatureH fet, QgsFeature& feature )
377+
bool QgsOgrFeatureIterator::readFeature( OGRFeatureH fet, QgsFeature& feature ) const
355378
{
356379
feature.setFeatureId( OGR_F_GetFID( fet ) );
357380
feature.initAttributes( mSource->mFields.count() );

‎src/providers/ogr/qgsogrfeatureiterator.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,10 @@ class QgsOgrFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsOgr
7070
//! fetch next feature filter expression
7171
bool nextFeatureFilterExpression( QgsFeature& f ) override;
7272

73-
bool readFeature( OGRFeatureH fet, QgsFeature& feature );
73+
bool readFeature( OGRFeatureH fet, QgsFeature& feature ) const;
7474

7575
//! Get an attribute associated with a feature
76-
void getFeatureAttribute( OGRFeatureH ogrFet, QgsFeature & f, int attindex );
76+
void getFeatureAttribute( OGRFeatureH ogrFet, QgsFeature & f, int attindex ) const;
7777

7878
bool mFeatureFetched;
7979

@@ -90,6 +90,10 @@ class QgsOgrFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsOgr
9090
QgsOgrAbstractGeometrySimplifier* mGeometrySimplifier;
9191

9292
bool mExpressionCompiled;
93+
QgsFeatureIds mFilterFids;
94+
QgsFeatureIds::const_iterator mFilterFidsIt;
95+
96+
bool fetchFeatureWithId( QgsFeatureId id, QgsFeature& feature ) const;
9397

9498
//! returns whether the iterator supports simplify geometries on provider side
9599
virtual bool providerCanSimplify( QgsSimplifyMethod::MethodType methodType ) const override;

‎tests/src/python/providertestbase.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,10 +352,36 @@ def testGetFeaturesFidsTests(self):
352352
expected = set([fids[1], fids[3], fids[4]])
353353
assert result == expected, 'Expected {} and got {} when testing for feature IDs filter'.format(expected, result)
354354

355+
#providers should ignore non-existant fids
356+
result = set([f.id() for f in self.provider.getFeatures(QgsFeatureRequest().setFilterFids([-101, fids[1], -102, fids[3], -103, fids[4], -104]))])
357+
expected = set([fids[1], fids[3], fids[4]])
358+
assert result == expected, 'Expected {} and got {} when testing for feature IDs filter'.format(expected, result)
359+
355360
result = set([f.id() for f in self.provider.getFeatures(QgsFeatureRequest().setFilterFids([]))])
356361
expected = set([])
357362
assert result == expected, 'Expected {} and got {} when testing for feature IDs filter'.format(expected, result)
358363

364+
# Rewind mid-way
365+
request = QgsFeatureRequest().setFilterFids([fids[1], fids[3], fids[4]])
366+
feature_it = self.provider.getFeatures(request)
367+
feature = QgsFeature()
368+
feature.setValid(True)
369+
self.assertTrue(feature_it.nextFeature(feature))
370+
self.assertIn(feature.id(), [fids[1], fids[3], fids[4]])
371+
first_feature = feature
372+
self.assertTrue(feature.isValid())
373+
# rewind
374+
self.assertTrue(feature_it.rewind())
375+
self.assertTrue(feature_it.nextFeature(feature))
376+
self.assertEqual(feature.id(), first_feature.id())
377+
self.assertTrue(feature.isValid())
378+
# grab all features
379+
self.assertTrue(feature_it.nextFeature(feature))
380+
self.assertTrue(feature_it.nextFeature(feature))
381+
# none left
382+
self.assertFalse(feature_it.nextFeature(feature))
383+
self.assertFalse(feature.isValid())
384+
359385
def testGetFeaturesFilterRectTests(self):
360386
extent = QgsRectangle(-70, 67, -60, 80)
361387
request = QgsFeatureRequest().setFilterRect(extent)

‎tests/src/python/test_qgsfeatureiterator.py

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,23 +81,20 @@ def test_FilterFids(self):
8181

8282
ids = [feat.id() for feat in pointLayer.getFeatures(QgsFeatureRequest().setFilterFids([7, 8, 12, 30]))]
8383
expectedIds = [7, 8, 12]
84-
myMessage = '\nExpected: {0} features\nGot: {1} features'.format(repr(expectedIds), repr(ids))
85-
assert ids == expectedIds, myMessage
84+
self.assertEquals(set(ids), set(expectedIds))
8685

8786
pointLayer.startEditing()
8887
self.addFeatures(pointLayer)
8988

9089
ids = [feat.id() for feat in pointLayer.getFeatures(QgsFeatureRequest().setFilterFids([-4, 7, 8, 12, 30]))]
9190
expectedIds = [-4, 7, 8, 12]
92-
myMessage = '\nExpected: {0} features\nGot: {1} features'.format(repr(expectedIds), repr(ids))
93-
assert ids == expectedIds, myMessage
91+
self.assertEquals(set(ids), set(expectedIds))
9492

9593
pointLayer.rollBack()
9694

9795
ids = [feat.id() for feat in pointLayer.getFeatures(QgsFeatureRequest().setFilterFids([-2, 7, 8, 12, 30]))]
9896
expectedIds = [7, 8, 12]
99-
myMessage = '\nExpected: {0} features\nGot: {1} features'.format(repr(expectedIds), repr(ids))
100-
assert ids == expectedIds, myMessage
97+
self.assertEquals(set(ids), set(expectedIds))
10198

10299
def addFeatures(self, vl):
103100
feat = QgsFeature()

0 commit comments

Comments
 (0)
Please sign in to comment.