Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix inconsistency between layer providers in geometry checker #58132

Closed
wants to merge 4 commits into from
Closed

Fix inconsistency between layer providers in geometry checker #58132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3393264
fix(qgsfeaturepool): cache management - fixes #58113
Djedouas Aug 1, 2024
57fce5a
fix(qgsfeaturepool): correctly remove features from spatial index
Djedouas Aug 6, 2024
cf0da99
fix(qgsfeaturepool): better synchronization between cache and spatial…
Djedouas Aug 27, 2024
542c10a
fix(qgsfeaturepool): fix locker mode
Djedouas Aug 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion python/PyQt6/analysis/auto_generated/vector/geometry_checker/qgsfeaturepool.sip.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,14 @@ To be used by implementations of ``addFeature``.
the original layer's thread.
%End

void refreshCache( const QgsFeature &feature );
void refreshCache( QgsFeature feature, const QgsFeature origFeature );
%Docstring
Changes a feature in the cache and the spatial index.
To be used by implementations of ``updateFeature``.

:param feature: the new feature to put in the cache and index
:param origFeature: the original feature to remove from the index

.. note::

This method can safely be called from a different thread vs the object's creation thread or
Expand Down
5 changes: 4 additions & 1 deletion python/analysis/auto_generated/vector/geometry_checker/qgsfeaturepool.sip.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,14 @@ To be used by implementations of ``addFeature``.
the original layer's thread.
%End

void refreshCache( const QgsFeature &feature );
void refreshCache( QgsFeature feature, const QgsFeature origFeature );
%Docstring
Changes a feature in the cache and the spatial index.
To be used by implementations of ``updateFeature``.

:param feature: the new feature to put in the cache and index
:param origFeature: the original feature to remove from the index

.. note::

This method can safely be called from a different thread vs the object's creation thread or
Expand Down
28 changes: 20 additions & 8 deletions src/analysis/vector/geometry_checker/qgsfeaturepool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ bool QgsFeaturePool::getFeature( QgsFeatureId id, QgsFeature &feature )
//
// https://bugreports.qt.io/browse/QTBUG-19794

QgsReadWriteLocker locker( mCacheLock, QgsReadWriteLocker::Write );
QgsReadWriteLocker locker( mCacheLock, QgsReadWriteLocker::Read );
QgsFeature *cachedFeature = mFeatureCache.object( id );
if ( cachedFeature )
{
Expand All @@ -66,7 +66,21 @@ bool QgsFeaturePool::getFeature( QgsFeatureId id, QgsFeature &feature )
}
locker.changeMode( QgsReadWriteLocker::Write );
mFeatureCache.insert( id, new QgsFeature( feature ) );
mIndex.addFeature( feature );

QgsGeometry indexGeom = mIndex.geometry( id );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The index isn't created with the FlagStoreFeatureGeometries flag, so this will always be null.

Can you please add some tests before trying more solutions? Right now we're just fumbling in the dark 🤣

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The index isn't created with the FlagStoreFeatureGeometries flag, so this will always be null.

The index creation here https://github.com/qgis/QGIS/blob/master/src/analysis/vector/geometry_checker/qgsfeaturepool.cpp#L82 has default flags which seems ON for FlagStoreFeatureGeometries https://github.com/qgis/QGIS/blob/master/src/core/qgsspatialindex.h#L77

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not how qflags work 😂

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...yes I was expecting that 😆 writing a test at the moment... ^^


// feature not in the index: add it
if ( indexGeom.isNull() )
mIndex.addFeature( feature );

// feature already in the index but with a different geometry (it has been evicted from the cache and externally modified):
// remove the old geometry and add the new one
else if ( !mIndex.geometry( id ).equals( feature.geometry() ) )
{
mIndex.deleteFeature( id, indexGeom.boundingBox() );
mIndex.addFeature( feature );
}

}
return true;
}
Expand Down Expand Up @@ -131,15 +145,13 @@ void QgsFeaturePool::insertFeature( const QgsFeature &feature, bool skipLock )
mIndex.addFeature( indexFeature );
}

void QgsFeaturePool::refreshCache( const QgsFeature &feature )
void QgsFeaturePool::refreshCache( QgsFeature feature, const QgsFeature origFeature )
{
QgsReadWriteLocker locker( mCacheLock, QgsReadWriteLocker::Write );
mFeatureCache.remove( feature.id() );
mIndex.deleteFeature( feature );
mFeatureCache.insert( feature.id(), new QgsFeature( feature ) );
mIndex.deleteFeature( origFeature );
mIndex.addFeature( feature );
locker.unlock();

QgsFeature tempFeature;
getFeature( feature.id(), tempFeature );
}

void QgsFeaturePool::removeFeature( const QgsFeatureId featureId )
Expand Down
5 changes: 4 additions & 1 deletion src/analysis/vector/geometry_checker/qgsfeaturepool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,13 @@ class ANALYSIS_EXPORT QgsFeaturePool : public QgsFeatureSink SIP_ABSTRACT
* Changes a feature in the cache and the spatial index.
* To be used by implementations of ``updateFeature``.
*
* \param feature the new feature to put in the cache and index
* \param origFeature the original feature to remove from the index
*
* \note This method can safely be called from a different thread vs the object's creation thread or
* the original layer's thread.
*/
void refreshCache( const QgsFeature &feature );
void refreshCache( QgsFeature feature, const QgsFeature origFeature );

/**
* Removes a feature from the cache and the spatial index.
Expand Down
2 changes: 1 addition & 1 deletion src/analysis/vector/geometry_checker/qgsvectordataproviderfeaturepool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ void QgsVectorDataProviderFeaturePool::updateFeature( QgsFeature &feature )
}
} );

refreshCache( feature );
refreshCache( feature, origFeature );
}

void QgsVectorDataProviderFeaturePool::deleteFeature( QgsFeatureId fid )
Expand Down
18 changes: 12 additions & 6 deletions src/analysis/vector/geometry_checker/qgsvectorlayerfeaturepool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ bool QgsVectorLayerFeaturePool::addFeatures( QgsFeatureList &features, QgsFeatur

void QgsVectorLayerFeaturePool::updateFeature( QgsFeature &feature )
{
QgsFeature origFeature;
getFeature( feature.id(), origFeature );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be getting confused here, but I think there's still a risk of index corruption in this scenario:

  1. Feature A is retrieved (version "1"), it is cached in the pool. Bounds are used in the spatial index.
  2. More features are fetched, cache fills up, feature A is evicted from cache
  3. Some external caller changes the geometry on feature A in the vector layer or data provider, now there's version 2 of feature A.
  4. A call to updateFeature for feature A is made. getFeature won't find it in the pool's cache (it was evicted), so it's retrieved from the layer. We get feature A version 2.
  5. Version 2 is passed to refreshCache, and the index is corrupted because we try to remove different bounds (version 2, whereas the cache is using v1)

I don't see any alternative but keeping a local map/hash of the feature bounds which were used in the index.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I did not expect that the spatial index could evict a feature... It is an index, not strictly a cache, isn't it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It won't get evicted from the spatial index, but will from mFeatureCache.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course 🙈 obviously.

Okay, what about "synchronizing" the cache and the index?

In fact, in the end of step 4. inside the index, we have both feature A version 2 and feature A version 1.

We could fix this: when getFeature doesn't find a cached feature, we can proactively delete any pre-existing corresponding feature in the index (see new commit).

If the feature was not present in the index, it does nearly nothing, otherwise it removes the possibly out-of-date feature from the index, to re-add the possibly same or up-to-date feature.


QgsThreadingUtils::runOnMainThread( [this, &feature]()
{
QgsVectorLayer *lyr = layer();
Expand All @@ -117,7 +120,7 @@ void QgsVectorLayerFeaturePool::updateFeature( QgsFeature &feature )
}
} );

refreshCache( feature );
refreshCache( feature, origFeature );
}

void QgsVectorLayerFeaturePool::deleteFeature( QgsFeatureId fid )
Expand All @@ -135,14 +138,17 @@ void QgsVectorLayerFeaturePool::deleteFeature( QgsFeatureId fid )

void QgsVectorLayerFeaturePool::onGeometryChanged( QgsFeatureId fid, const QgsGeometry &geometry )
{
Q_UNUSED( geometry )

QgsFeature feature;
if ( isFeatureCached( fid ) )
{
QgsFeature feature;
getFeature( fid, feature );
refreshCache( feature );
QgsFeature origFeature;
getFeature( fid, origFeature ); // gets the cached feature
feature = origFeature;
feature.setGeometry( geometry );
refreshCache( feature, origFeature );
}
else
getFeature( fid, feature ); // gets the feature and adds it to the cache and index
}

void QgsVectorLayerFeaturePool::onFeatureDeleted( QgsFeatureId fid )
Expand Down
102 changes: 100 additions & 2 deletions tests/src/geometry_checker/testqgsgeometrychecks.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ class TestQgsGeometryChecks: public QObject
};
double layerToMapUnits( const QgsMapLayer *layer, const QgsCoordinateReferenceSystem &mapCrs ) const;
QgsFeaturePool *createFeaturePool( QgsVectorLayer *layer, bool selectedOnly = false ) const;
QPair<QgsGeometryCheckContext *, QMap<QString, QgsFeaturePool *> > createTestContext( QTemporaryDir &tempDir, QMap<QString, QString> &layers, const QgsCoordinateReferenceSystem &mapCrs = QgsCoordinateReferenceSystem( "EPSG:4326" ), double prec = 8 ) const;
QPair<QgsGeometryCheckContext *, QMap<QString, QgsFeaturePool *> > createTestContext( QTemporaryDir &tempDir, QMap<QString, QString> &layers, const QgsCoordinateReferenceSystem &mapCrs = QgsCoordinateReferenceSystem( "EPSG:4326" ), int prec = 8 ) const;
QPair<QgsGeometryCheckContext *, QMap<QString, QgsFeaturePool *> > createMemoryTestContext( QMap<QString, QString> &layers, const QgsCoordinateReferenceSystem &mapCrs = QgsCoordinateReferenceSystem( "EPSG:4326" ), int prec = 8 ) const;
void cleanupTestContext( QPair<QgsGeometryCheckContext *, QMap<QString, QgsFeaturePool *> > ctx ) const;
void listErrors( const QList<QgsGeometryCheckError *> &checkErrors, const QStringList &messages ) const;
QList<QgsGeometryCheckError *> searchCheckErrors( const QList<QgsGeometryCheckError *> &checkErrors, const QString &layerId, const QgsFeatureId &featureId = -1, const QgsPointXY &pos = QgsPointXY(), const QgsVertexId &vid = QgsVertexId(), const QVariant &value = QVariant(), double tol = 1E-4 ) const;
Expand All @@ -78,6 +79,7 @@ class TestQgsGeometryChecks: public QObject

private slots:
void testAngleCheck();
void testAngleCheckMemoryLayers();
void testAreaCheck();
void testContainedCheck();
void testDangleCheck();
Expand Down Expand Up @@ -117,6 +119,85 @@ void TestQgsGeometryChecks::cleanupTestCase()

///////////////////////////////////////////////////////////////////////////////

void TestQgsGeometryChecks::testAngleCheckMemoryLayers()
{
QTemporaryDir dir;
QMap<QString, QString> layers;
layers.insert( "point_layer.shp", "" );
layers.insert( "line_layer.shp", "" );
layers.insert( "polygon_layer.shp", "" );
auto testContext = createMemoryTestContext( layers );

// Test detection
QList<QgsGeometryCheckError *> checkErrors;
QStringList messages;

QVariantMap configuration;
configuration.insert( "minAngle", 15 );

const QgsGeometryAngleCheck check( testContext.first, configuration );
QgsFeedback feedback;
check.collectErrors( testContext.second, checkErrors, messages, &feedback );
listErrors( checkErrors, messages );

QList<QgsGeometryCheckError *> errs1;
QList<QgsGeometryCheckError *> errs2;

QCOMPARE( checkErrors.size(), 8 );
QVERIFY( searchCheckErrors( checkErrors, layers["point_layer.shp"] ).isEmpty() );
QVERIFY( ( errs1 = searchCheckErrors( checkErrors, layers["line_layer.shp"], 1, QgsPointXY( -0.2225, 0.5526 ), QgsVertexId( 0, 0, 3 ), 10.5865 ) ).size() == 1 );
QVERIFY( searchCheckErrors( checkErrors, layers["line_layer.shp"], 1, QgsPointXY( -0.94996, 0.99967 ), QgsVertexId( 1, 0, 1 ), 8.3161 ).size() == 1 );
QVERIFY( searchCheckErrors( checkErrors, layers["line_layer.shp"], 3, QgsPointXY( -0.4547, -0.3059 ), QgsVertexId( 0, 0, 1 ), 5.4165 ).size() == 1 );
QVERIFY( searchCheckErrors( checkErrors, layers["line_layer.shp"], 3, QgsPointXY( -0.7594, -0.1971 ), QgsVertexId( 0, 0, 2 ), 12.5288 ).size() == 1 );
QVERIFY( ( errs2 = searchCheckErrors( checkErrors, layers["polygon_layer.shp"], 1, QgsPointXY( 0.2402, 1.0786 ), QgsVertexId( 0, 0, 1 ), 13.5140 ) ).size() == 1 );
QVERIFY( searchCheckErrors( checkErrors, layers["polygon_layer.shp"], 2, QgsPointXY( 0.6960, 0.5908 ), QgsVertexId( 0, 0, 0 ), 7.0556 ).size() == 1 );
QVERIFY( searchCheckErrors( checkErrors, layers["polygon_layer.shp"], 2, QgsPointXY( 0.98690, 0.55699 ), QgsVertexId( 1, 0, 5 ), 7.7351 ).size() == 1 );
QVERIFY( searchCheckErrors( checkErrors, layers["polygon_layer.shp"], 12, QgsPointXY( -0.3186, 1.6734 ), QgsVertexId( 0, 0, 1 ), 3.5092 ).size() == 1 );

// Test fixes
QgsFeature f;
int n1, n2;

testContext.second[errs1[0]->layerId()]->getFeature( errs1[0]->featureId(), f );
n1 = f.geometry().constGet()->vertexCount( errs1[0]->vidx().part, errs1[0]->vidx().ring );
QVERIFY( fixCheckError( testContext.second, errs1[0],
QgsGeometryAngleCheck::DeleteNode, QgsGeometryCheckError::StatusFixed,
{{errs1[0]->layerId(), errs1[0]->featureId(), QgsGeometryCheck::ChangeNode, QgsGeometryCheck::ChangeRemoved, errs1[0]->vidx()}} ) );
testContext.second[errs1[0]->layerId()]->getFeature( errs1[0]->featureId(), f );
n2 = f.geometry().constGet()->vertexCount( errs1[0]->vidx().part, errs1[0]->vidx().ring );
QCOMPARE( n1, n2 + 1 );

testContext.second[errs2[0]->layerId()]->getFeature( errs2[0]->featureId(), f );
n1 = f.geometry().constGet()->vertexCount( errs2[0]->vidx().part, errs2[0]->vidx().ring );
QVERIFY( fixCheckError( testContext.second, errs2[0],
QgsGeometryAngleCheck::DeleteNode, QgsGeometryCheckError::StatusFixed,
{{errs2[0]->layerId(), errs2[0]->featureId(), QgsGeometryCheck::ChangeNode, QgsGeometryCheck::ChangeRemoved, errs2[0]->vidx()}} ) );
testContext.second[errs2[0]->layerId()]->getFeature( errs2[0]->featureId(), f );
n2 = f.geometry().constGet()->vertexCount( errs2[0]->vidx().part, errs2[0]->vidx().ring );
QCOMPARE( n1, n2 + 1 );

// Test change tracking
QVERIFY( !errs2[0]->handleChanges( change2changes( {errs2[0]->layerId(), errs2[0]->featureId(), QgsGeometryCheck::ChangeFeature, QgsGeometryCheck::ChangeRemoved, QgsVertexId()} ) ) );
QVERIFY( !errs2[0]->handleChanges( change2changes( {errs2[0]->layerId(), errs2[0]->featureId(), QgsGeometryCheck::ChangeFeature, QgsGeometryCheck::ChangeChanged, QgsVertexId()} ) ) );
QVERIFY( !errs2[0]->handleChanges( change2changes( {errs2[0]->layerId(), errs2[0]->featureId(), QgsGeometryCheck::ChangePart, QgsGeometryCheck::ChangeRemoved, QgsVertexId( errs2[0]->vidx().part )} ) ) );
QVERIFY( !errs2[0]->handleChanges( change2changes( {errs2[0]->layerId(), errs2[0]->featureId(), QgsGeometryCheck::ChangePart, QgsGeometryCheck::ChangeChanged, QgsVertexId( errs2[0]->vidx().part )} ) ) );
QVERIFY( errs2[0]->handleChanges( change2changes( {errs2[0]->layerId(), errs2[0]->featureId(), QgsGeometryCheck::ChangePart, QgsGeometryCheck::ChangeRemoved, QgsVertexId( errs2[0]->vidx().part + 1 )} ) ) );
QVERIFY( errs2[0]->handleChanges( change2changes( {errs2[0]->layerId(), errs2[0]->featureId(), QgsGeometryCheck::ChangePart, QgsGeometryCheck::ChangeChanged, QgsVertexId( errs2[0]->vidx().part + 1 )} ) ) );
QVERIFY( !errs2[0]->handleChanges( change2changes( {errs2[0]->layerId(), errs2[0]->featureId(), QgsGeometryCheck::ChangeRing, QgsGeometryCheck::ChangeRemoved, QgsVertexId( errs2[0]->vidx().part, errs2[0]->vidx().ring )} ) ) );
QVERIFY( !errs2[0]->handleChanges( change2changes( {errs2[0]->layerId(), errs2[0]->featureId(), QgsGeometryCheck::ChangeRing, QgsGeometryCheck::ChangeChanged, QgsVertexId( errs2[0]->vidx().part, errs2[0]->vidx().ring )} ) ) );
QVERIFY( errs2[0]->handleChanges( change2changes( {errs2[0]->layerId(), errs2[0]->featureId(), QgsGeometryCheck::ChangeRing, QgsGeometryCheck::ChangeRemoved, QgsVertexId( errs2[0]->vidx().part, errs2[0]->vidx().ring + 1 )} ) ) );
QVERIFY( errs2[0]->handleChanges( change2changes( {errs2[0]->layerId(), errs2[0]->featureId(), QgsGeometryCheck::ChangeRing, QgsGeometryCheck::ChangeChanged, QgsVertexId( errs2[0]->vidx().part, errs2[0]->vidx().ring + 1 )} ) ) );
QVERIFY( !errs2[0]->handleChanges( change2changes( {errs2[0]->layerId(), errs2[0]->featureId(), QgsGeometryCheck::ChangeNode, QgsGeometryCheck::ChangeRemoved, errs2[0]->vidx()} ) ) );
QVERIFY( !errs2[0]->handleChanges( change2changes( {errs2[0]->layerId(), errs2[0]->featureId(), QgsGeometryCheck::ChangeNode, QgsGeometryCheck::ChangeChanged, errs2[0]->vidx()} ) ) );
QVERIFY( errs2[0]->handleChanges( change2changes( {errs2[0]->layerId(), errs2[0]->featureId(), QgsGeometryCheck::ChangeNode, QgsGeometryCheck::ChangeRemoved, QgsVertexId( errs2[0]->vidx().part, errs2[0]->vidx().ring, errs2[0]->vidx().vertex + 1 )} ) ) );
QVERIFY( errs2[0]->handleChanges( change2changes( {errs2[0]->layerId(), errs2[0]->featureId(), QgsGeometryCheck::ChangeNode, QgsGeometryCheck::ChangeChanged, QgsVertexId( errs2[0]->vidx().part, errs2[0]->vidx().ring, errs2[0]->vidx().vertex + 1 )} ) ) );
const QgsVertexId oldVidx = errs2[0]->vidx();
QVERIFY( errs2[0]->handleChanges( change2changes( {errs2[0]->layerId(), errs2[0]->featureId(), QgsGeometryCheck::ChangeNode, QgsGeometryCheck::ChangeRemoved, QgsVertexId( errs2[0]->vidx().part, errs2[0]->vidx().ring, errs2[0]->vidx().vertex - 1 )} ) ) );
QVERIFY( errs2[0]->vidx().vertex == oldVidx.vertex - 1 );

cleanupTestContext( testContext );
}

void TestQgsGeometryChecks::testAngleCheck()
{
QTemporaryDir dir;
Expand Down Expand Up @@ -1328,7 +1409,24 @@ QgsFeaturePool *TestQgsGeometryChecks::createFeaturePool( QgsVectorLayer *layer,
return new QgsVectorDataProviderFeaturePool( layer, selectedOnly );
}

QPair<QgsGeometryCheckContext *, QMap<QString, QgsFeaturePool *> > TestQgsGeometryChecks::createTestContext( QTemporaryDir &tempDir, QMap<QString, QString> &layers, const QgsCoordinateReferenceSystem &mapCrs, double prec ) const
QPair<QgsGeometryCheckContext *, QMap<QString, QgsFeaturePool *> > TestQgsGeometryChecks::createMemoryTestContext( QMap<QString, QString> &layers, const QgsCoordinateReferenceSystem &mapCrs, int prec ) const
{
const QDir testDataDir( QDir( TEST_DATA_DIR ).absoluteFilePath( "geometry_checker" ) );

QMap<QString, QgsFeaturePool *> featurePools;
for ( auto it = layers.begin(); it != layers.end(); it++ )
{
const QString layerFile = it.key();
QgsVectorLayer *layer = ( new QgsVectorLayer( testDataDir.absoluteFilePath( layerFile ), layerFile ) )->materialize( QgsFeatureRequest() );
Q_ASSERT( layer && layer->isValid() );
layers[layerFile] = layer->id();
layer->dataProvider()->enterUpdateMode();
featurePools.insert( layer->id(), createFeaturePool( layer ) );
}
return qMakePair( new QgsGeometryCheckContext( prec, mapCrs, QgsProject::instance()->transformContext(), QgsProject::instance() ), featurePools );
}

QPair<QgsGeometryCheckContext *, QMap<QString, QgsFeaturePool *> > TestQgsGeometryChecks::createTestContext( QTemporaryDir &tempDir, QMap<QString, QString> &layers, const QgsCoordinateReferenceSystem &mapCrs, int prec ) const
{
const QDir testDataDir( QDir( TEST_DATA_DIR ).absoluteFilePath( "geometry_checker" ) );
const QDir tmpDir( tempDir.path() );
Expand Down
22 changes: 7 additions & 15 deletions tests/src/geometry_checker/testqgsvectorlayerfeaturepool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,29 +142,21 @@ void TestQgsVectorLayerFeaturePool::changeGeometry()
feat.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "Polygon((100 100, 110 100, 110 110, 100 110, 100 100))" ) ) );
vl->updateFeature( feat );

// Still working on the cached data
// Cached data updated with geometryChanged vector layer signal
const QgsFeatureIds ids3 = pool.getIntersects( QgsRectangle( 0, 0, 10, 10 ) );
QCOMPARE( ids3.size(), 1 );

// Repopulate the cache
const QgsFeatureIds ids4 = pool.getFeatures( QgsFeatureRequest().setFilterRect( QgsRectangle( 0, 0, 10, 10 ) ) );
QCOMPARE( ids4.size(), 0 );
QCOMPARE( ids3.size(), 0 );

// Still working on the cached data
const QgsFeatureIds ids5 = pool.getIntersects( QgsRectangle( 0, 0, 10, 10 ) );
QCOMPARE( ids5.size(), 0 );
const QgsFeatureIds ids4 = pool.getIntersects( QgsRectangle( 0, 0, 10, 10 ) );
QCOMPARE( ids4.size(), 0 );

// Update a feature to be inside the AOI
feat.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "Polygon((0 0, 10 0, 10 10, 0 10, 0 0))" ) ) );
vl->updateFeature( feat );

// Still cached
const QgsFeatureIds ids6 = pool.getIntersects( QgsRectangle( 0, 0, 10, 10 ) );
QCOMPARE( ids6.size(), 0 );

// One in there again
const QgsFeatureIds ids7 = pool.getFeatures( QgsFeatureRequest().setFilterRect( QgsRectangle( 0, 0, 10, 10 ) ) );
QCOMPARE( ids7.size(), 1 );
// Cached data updated with geometryChanged vector layer signal
const QgsFeatureIds ids5 = pool.getIntersects( QgsRectangle( 0, 0, 10, 10 ) );
QCOMPARE( ids5.size(), 1 );
}

std::unique_ptr<QgsVectorLayer> TestQgsVectorLayerFeaturePool::createPopulatedLayer()
Expand Down
Loading